Changeset 89916 in webkit


Ignore:
Timestamp:
Jun 28, 2011 2:52:10 AM (13 years ago)
Author:
apavlov@chromium.org
Message:

2011-06-27 Alexander Pavlov <apavlov@chromium.org>

Reviewed by Kent Tamura.

[Chromium] SELECT or autofill popup is trimmed by screen edge on Windows
https://bugs.webkit.org/show_bug.cgi?id=63438

If the popup is calculated to be trimmed by a screen edge, an attempt is made
to alter its vertical edge alignment (set to the right edge for LTR or to the left edge for RTL)
to see if the trimmed portion becomes smaller than that with the original layout.
The change involves remembering the original frameRect for the popup and restoring it in refresh().
This is due to the fact that the frameRect originally set in showInRect() is overwritten
by layoutAndGetRTLOffset(), which breaks the originally requested popup container layout metrics.
The max width is reset on every layoutAndCalculateWidgetRect(), as it can be constrained by the screen edge,
and thus should be re-checked every time the popup is displayed, in case the browser window has been moved.

Tests: manual-tests/popup-width-restriction-within-screen.html partly covers the fix (should not regress).
Other than that, there is no way to unit-test the platform-specific native code.

  • platform/chromium/PopupMenuChromium.cpp: (WebCore::PopupListBox::setMaxWidth): Added. Avoid duplicate popup content layouts (in contrast with setMaxWidthAndLayout()). (WebCore::PopupListBox::PopupListBox): (WebCore::PopupContainer::layoutAndCalculateWidgetRect): Attempt a left-right popup alignment inversion to minimize the trimmed content. Also restore a default max popup width. (WebCore::PopupContainer::layoutAndGetRTLOffset): Always return the popup listbox offset for the RTL (right alignment) case. The method rename is due to the return value semantics change. (WebCore::PopupContainer::showInRect): Store the originally requested frameRect for the popup. (WebCore::PopupContainer::refresh): Restore the original popup frameRect to avoid layout artifacts on refresh. (WebCore::PopupContainer::isRTL): This check should be made by the layoutAndGetRTLOffset() clients.
  • platform/chromium/PopupMenuChromium.h:
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r89915 r89916  
     12011-06-27  Alexander Pavlov  <apavlov@chromium.org>
     2
     3        Reviewed by Kent Tamura.
     4
     5        [Chromium] SELECT or autofill popup is trimmed by screen edge on Windows
     6        https://bugs.webkit.org/show_bug.cgi?id=63438
     7
     8        If the popup is calculated to be trimmed by a screen edge, an attempt is made
     9        to alter its vertical edge alignment (set to the right edge for LTR or to the left edge for RTL)
     10        to see if the trimmed portion becomes smaller than that with the original layout.
     11        The change involves remembering the original frameRect for the popup and restoring it in refresh().
     12        This is due to the fact that the frameRect originally set in showInRect() is overwritten
     13        by layoutAndGetRTLOffset(), which breaks the originally requested popup container layout metrics.
     14        The max width is reset on every layoutAndCalculateWidgetRect(), as it can be constrained by the screen edge,
     15        and thus should be re-checked every time the popup is displayed, in case the browser window has been moved.
     16
     17        Tests: manual-tests/popup-width-restriction-within-screen.html partly covers the fix (should not regress).
     18        Other than that, there is no way to unit-test the platform-specific native code.
     19
     20        * platform/chromium/PopupMenuChromium.cpp:
     21        (WebCore::PopupListBox::setMaxWidth): Added. Avoid duplicate popup content layouts (in contrast with setMaxWidthAndLayout()).
     22        (WebCore::PopupListBox::PopupListBox):
     23        (WebCore::PopupContainer::layoutAndCalculateWidgetRect): Attempt a left-right popup alignment inversion
     24        to minimize the trimmed content. Also restore a default max popup width.
     25        (WebCore::PopupContainer::layoutAndGetRTLOffset): Always return the popup listbox offset for the RTL (right alignment) case.
     26        The method rename is due to the return value semantics change.
     27        (WebCore::PopupContainer::showInRect): Store the originally requested frameRect for the popup.
     28        (WebCore::PopupContainer::refresh): Restore the original popup frameRect to avoid layout artifacts on refresh.
     29        (WebCore::PopupContainer::isRTL): This check should be made by the layoutAndGetRTLOffset() clients.
     30        * platform/chromium/PopupMenuChromium.h:
     31
    1322011-06-28  Kentaro Hara  <haraken@google.com>
    233
  • trunk/Source/WebCore/platform/chromium/PopupMenuChromium.cpp

    r88895 r89916  
    5959#include <wtf/unicode/CharacterNames.h>
    6060
     61using namespace std;
    6162using namespace WTF;
    6263using namespace Unicode;
     
    146147
    147148    void setMaxHeight(int maxHeight) { m_maxHeight = maxHeight; }
     149
     150    void setMaxWidth(int maxWidth) { m_maxWindowWidth = maxWidth; }
    148151
    149152    void setMaxWidthAndLayout(int);
     
    168171        , m_repeatingChar(0)
    169172        , m_lastCharTime(0)
    170         , m_maxWindowWidth(std::numeric_limits<int>::max())
     173        , m_maxWindowWidth(numeric_limits<int>::max())
    171174    {
    172175        setScrollbarModes(ScrollbarAlwaysOff, ScrollbarAlwaysOff);
     
    341344IntRect PopupContainer::layoutAndCalculateWidgetRect(int targetControlHeight, const IntPoint& popupInitialCoordinate)
    342345{
    343     // Reset the max height to its default value, it will be recomputed below
     346    // Reset the max width and height to their default values, they will be recomputed below
    344347    // if necessary.
    345348    m_listBox->setMaxHeight(kMaxHeight);
     349    m_listBox->setMaxWidth(numeric_limits<int>::max());
    346350
    347351    // Lay everything out to figure out our preferred size, then tell the view's
    348352    // WidgetClient about it.  It should assign us a client.
    349     int rightOffset = layoutAndGetRightOffset();
     353    int rtlOffset = layoutAndGetRTLOffset();
     354    bool isRTL = this->isRTL();
     355    int rightOffset = isRTL ? rtlOffset : 0;
    350356
    351357    // Assume m_listBox size is already calculated.
     
    367373        // When clipping, we also need to set a maximum width for the list box.
    368374        FloatRect windowRect = chromeClient->windowRect();
    369         if (windowRect.x() >= screen.x() && windowRect.maxX() <= screen.maxX()) {
    370             if (m_listBox->m_popupClient->menuStyle().textDirection() == RTL && widgetRect.x() < screen.x()) {
     375        if (windowRect.x() >= screen.x() && windowRect.maxX() <= screen.maxX() && (widgetRect.x() < screen.x() || widgetRect.maxX() > screen.maxX())) {
     376            // First, inverse the popup alignment if it does not fit the screen - this might fix things (or make them better).
     377            IntRect inverseWidgetRect = chromeClient->windowToScreen(IntRect(popupInitialCoordinate.x() + (isRTL ? 0 : rtlOffset), popupInitialCoordinate.y(), targetSize.width(), targetSize.height()));
     378            IntRect enclosingScreen = enclosingIntRect(screen);
     379            unsigned originalCutoff = max(enclosingScreen.x() - widgetRect.x(), 0) + max(widgetRect.maxX() - enclosingScreen.maxX(), 0);
     380            unsigned inverseCutoff = max(enclosingScreen.x() - inverseWidgetRect.x(), 0) + max(inverseWidgetRect.maxX() - enclosingScreen.maxX(), 0);
     381
     382            // Accept the inverse popup alignment if the trimmed content gets shorter than that in the original alignment case.
     383            if (inverseCutoff < originalCutoff)
     384                widgetRect = inverseWidgetRect;
     385
     386            if (widgetRect.x() < screen.x()) {
     387                unsigned widgetRight = widgetRect.maxX();
    371388                widgetRect.setWidth(widgetRect.maxX() - screen.x());
    372                 widgetRect.setX(screen.x());
     389                widgetRect.setX(widgetRight - widgetRect.width());
    373390                listBox()->setMaxWidthAndLayout(max(widgetRect.width() - kBorderSize * 2, 0));
    374391            } else if (widgetRect.maxX() > screen.maxX()) {
     
    392409                else
    393410                    m_listBox->setMaxHeight(spaceBelow);
    394                 layoutAndGetRightOffset();
     411                layoutAndGetRTLOffset();
    395412                // Our height has changed, so recompute only Y axis of widgetRect.
    396413                // We don't have to recompute X axis, so we only replace Y axis
     
    445462}
    446463
    447 int PopupContainer::layoutAndGetRightOffset()
     464int PopupContainer::layoutAndGetRTLOffset()
    448465{
    449466    m_listBox->layout();
     
    457474    int listBoxWidth = m_listBox->width() + kBorderSize * 2;
    458475    resize(listBoxWidth, m_listBox->height() + kBorderSize * 2);
    459 
    460     // Adjust the starting x-axis for RTL dropdown. For RTL dropdown, the right edge
    461     // of dropdown box should be aligned with the right edge of <select> element box,
    462     // and the dropdown box should be expanded to left if more space needed.
    463     PopupMenuClient* popupClient = m_listBox->m_popupClient;
    464     int rightOffset = 0;
    465     if (popupClient) {
    466         bool rightAligned = m_listBox->m_popupClient->menuStyle().textDirection() == RTL;
    467         if (rightAligned)
    468             rightOffset = popupWidth - listBoxWidth;
    469     }
    470476    invalidate();
    471477
    472     return rightOffset;
     478    // Compute the starting x-axis for a normal RTL or right-aligned LTR dropdown. For those,
     479    // the right edge of dropdown box should be aligned with the right edge of <select> element box,
     480    // and the dropdown box should be expanded to the left if more space is needed.
     481    return popupWidth - listBoxWidth;
    473482}
    474483
     
    576585    location.move(0, r.height());
    577586
    578     setFrameRect(IntRect(location, r.size()));
     587    m_originalFrameRect = IntRect(location, r.size());
     588    setFrameRect(m_originalFrameRect);
    579589    showPopup(v);
    580590}
     
    585595    // Move it below the select widget.
    586596    location.move(0, targetControlRect.height());
     597
     598    listBox()->setBaseWidth(max(m_originalFrameRect.width() - kBorderSize * 2, 0));
     599    setFrameRect(m_originalFrameRect);
    587600
    588601    listBox()->updateFromElement();
     
    590603    IntSize originalSize = size();
    591604    IntRect widgetRect = layoutAndCalculateWidgetRect(targetControlRect.height(), location);
    592     if (originalSize != widgetRect.size())
    593         setFrameRect(widgetRect);
     605    if (originalSize != widgetRect.size()) {
     606        ChromeClientChromium* chromeClient = chromeClientChromium();
     607        if (chromeClient) {
     608            IntPoint widgetLocation = chromeClient->screenToWindow(widgetRect.location());
     609            widgetRect.setLocation(widgetLocation);
     610            setFrameRect(widgetRect);
     611        }
     612    }
    594613
    595614    invalidate();
     615}
     616
     617inline bool PopupContainer::isRTL() const
     618{
     619    return m_listBox->m_popupClient->menuStyle().textDirection() == RTL;
    596620}
    597621
  • trunk/Source/WebCore/platform/chromium/PopupMenuChromium.h

    r88021 r89916  
    142142    void notifyPopupHidden();
    143143
    144     // Compute size of widget and children. Return right offset for RTL.
    145     int layoutAndGetRightOffset();
     144    // Compute size of widget and children. Return right offset for the popup right alignment.
     145    int layoutAndGetRTLOffset();
    146146
    147147    PopupListBox* listBox() const { return m_listBox.get(); }
     148
     149    bool isRTL() const;
    148150
    149151    // Gets the index of the item that the user is currently moused-over or
     
    191193    PopupContainerSettings m_settings;
    192194    PopupType m_popupType;
     195    IntRect m_originalFrameRect;
    193196    // Whether the popup is currently open.
    194197    bool m_popupOpen;
Note: See TracChangeset for help on using the changeset viewer.