Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(238)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10267006: Merge 113418 (Closed)

Created:
8 years, 7 months ago by Chris Evans
Modified:
8 years, 7 months ago
Reviewers:
cevans, apavlov
CC:
chromium-reviews
Base URL:
http://svn.webkit.org/repository/webkit/branches/chromium/1084/
Visibility:
Public.

Description

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -40 lines) Patch
M Source/WebCore/platform/chromium/PopupContainer.h View 1 chunk +6 lines, -0 lines 0 comments Download
M Source/WebCore/platform/chromium/PopupContainer.cpp View 5 chunks +45 lines, -40 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Chris Evans
8 years, 7 months ago (2012-04-30 08:56:22 UTC) #1
apavlov
Hey Chris, This merge should definitely be followed by the merge of http://trac.webkit.org/changeset/114513 (for https://bugs.webkit.org/show_bug.cgi?id=84139, ...
8 years, 7 months ago (2012-04-30 16:37:53 UTC) #2
cevans
8 years, 7 months ago (2012-04-30 20:57:44 UTC) #3 
On Mon, Apr 30, 2012 at 10:37 AM, Alexander Pavlov <apavlov@chromium.org>wrote:

> Hey Chris,
>
> This merge should definitely be followed by the merge of
> http://trac.webkit.org/changeset/114513 (for
> https://bugs.webkit.org/show_bug.cgi?id=84139,
> http://code.google.com/p/chromium/issues/detail?id=123432 downstream).
> Otherwise it results in quite unexpected behavior in certain cases. Could
> you handle this as well?
>

Thank you for your vigilence. I'll merge this additional CL immediately and
update the bug.


>
>
> On Mon, Apr 30, 2012 at 12:56 PM, <cevans@chromium.org> wrote:
>
>> Reviewers: apavlov,
>>
>> Description:
>> Merge 113418
>> BUG=118374
>>
>> Please review this at
https://chromiumcodereview.**appspot.com/10267006/<https://chromiumcodereview...
>>
>> SVN Base: http://svn.webkit.org/**repository/webkit/branches/**
>>
chromium/1084/<http://svn.webkit.org/repository/webkit/branches/chromium/1084/>
>>
>> Affected files:
>>  M     Source/WebCore/platform/**chromium/PopupContainer.h
>>  M     Source/WebCore/platform/**chromium/PopupContainer.cpp
>>
>>
>> Index: Source/WebCore/platform/**chromium/PopupContainer.cpp
>> ==============================**==============================**=======
>> --- Source/WebCore/platform/**chromium/PopupContainer.cpp (revision
>> 115617)
>> +++ Source/WebCore/platform/**chromium/PopupContainer.cpp (working copy)
>> @@ -134,7 +134,7 @@
>>     IntSize targetSize(m_listBox->width() + kBorderSize * 2,
>>                        m_listBox->height() + kBorderSize * 2);
>>
>> -    IntRect widgetRect;
>> +    IntRect widgetRectInScreen;
>>     ChromeClientChromium* chromeClient = chromeClientChromium();
>>     if (chromeClient) {
>>         // If the popup would extend past the bottom of the screen, open
>> upwards
>> @@ -142,44 +142,44 @@
>>         FloatRect screen = screenAvailableRect(m_**frameView.get());
>>         // Use popupInitialCoordinate.x() + rightOffset because RTL
>> position
>>         // needs to be considered.
>> -        widgetRect =
chromeClient->**rootViewToScreen(IntRect(**popupInitialCoordinate.x()
>> + rightOffset, popupInitialCoordinate.y(), targetSize.width(),
>> targetSize.height()));
>> +        widgetRectInScreen =
chromeClient->**rootViewToScreen(IntRect(**popupInitialCoordinate.x()
>> + rightOffset, popupInitialCoordinate.y(), targetSize.width(),
>> targetSize.height()));
>>
>>         // If we have multiple screens and the browser rect is in one
>> screen, we have
>>         // to clip the window width to the screen width.
>>         // When clipping, we also need to set a maximum width for the
>> list box.
>>         FloatRect windowRect = chromeClient->windowRect();
>> -        if (windowRect.x() >= screen.x() && windowRect.maxX() <=
>> screen.maxX() && (widgetRect.x() < screen.x() || widgetRect.maxX() >
>> screen.maxX())) {
>> +        if (windowRect.x() >= screen.x() && windowRect.maxX() <=
>> screen.maxX() && (widgetRectInScreen.x() < screen.x() ||
>> widgetRectInScreen.maxX() > screen.maxX())) {
>>             // First, inverse the popup alignment if it does not fit the
>> screen - this might fix things (or make them better).
>> -            IntRect inverseWidgetRect = chromeClient->**
>> rootViewToScreen(IntRect(**popupInitialCoordinate.x() + (isRTL ? 0 :
>> rtlOffset), popupInitialCoordinate.y(), targetSize.width(),
>> targetSize.height()));
>> +            IntRect inverseWidgetRectInScreen = chromeClient->**
>> rootViewToScreen(IntRect(**popupInitialCoordinate.x() + (isRTL ? 0 :
>> rtlOffset), popupInitialCoordinate.y(), targetSize.width(),
>> targetSize.height()));
>>             IntRect enclosingScreen = enclosingIntRect(screen);
>> -            unsigned originalCutoff = max(enclosingScreen.x() -
>> widgetRect.x(), 0) + max(widgetRect.maxX() - enclosingScreen.maxX(), 0);
>> -            unsigned inverseCutoff = max(enclosingScreen.x() -
>> inverseWidgetRect.x(), 0) + max(inverseWidgetRect.maxX() -
>> enclosingScreen.maxX(), 0);
>> +            unsigned originalCutoff = max(enclosingScreen.x() -
>> widgetRectInScreen.x(), 0) + max(widgetRectInScreen.maxX() -
>> enclosingScreen.maxX(), 0);
>> +            unsigned inverseCutoff = max(enclosingScreen.x() -
>> inverseWidgetRectInScreen.x(), 0) + max(inverseWidgetRectInScreen.**maxX()
>> - enclosingScreen.maxX(), 0);
>>
>>             // Accept the inverse popup alignment if the trimmed content
>> gets shorter than that in the original alignment case.
>>             if (inverseCutoff < originalCutoff)
>> -                widgetRect = inverseWidgetRect;
>> +              widgetRectInScreen = inverseWidgetRectInScreen;
>>
>> -            if (widgetRect.x() < screen.x()) {
>> -                unsigned widgetRight = widgetRect.maxX();
>> -                widgetRect.setWidth(**widgetRect.maxX() - screen.x());
>> -                widgetRect.setX(widgetRight - widgetRect.width());
>> -                listBox()->**setMaxWidthAndLayout(max(**widgetRect.width()
>> - kBorderSize * 2, 0));
>> -            } else if (widgetRect.maxX() > screen.maxX()) {
>> -                widgetRect.setWidth(screen.**maxX() - widgetRect.x());
>> -                listBox()->**setMaxWidthAndLayout(max(**widgetRect.width()
>> - kBorderSize * 2, 0));
>> +            if (widgetRectInScreen.x() < screen.x()) {
>> +                unsigned widgetRight = widgetRectInScreen.maxX();
>> +                widgetRectInScreen.setWidth(**widgetRectInScreen.maxX()
>> - screen.x());
>> +                widgetRectInScreen.setX(**widgetRight -
>> widgetRectInScreen.width());
>> +               
listBox()->**setMaxWidthAndLayout(max(**widgetRectInScreen.width()
>> - kBorderSize * 2, 0));
>> +            } else if (widgetRectInScreen.maxX() > screen.maxX()) {
>> +                widgetRectInScreen.setWidth(**screen.maxX() -
>> widgetRectInScreen.x());
>> +               
listBox()->**setMaxWidthAndLayout(max(**widgetRectInScreen.width()
>> - kBorderSize * 2, 0));
>>             }
>>         }
>>
>>         // Calculate Y axis size.
>> -        if (widgetRect.maxY() > static_cast<int>(screen.maxY()**)) {
>> -            if (widgetRect.y() - widgetRect.height() -
>> targetControlHeight
>>
>>> 0) {
>>>
>> +        if (widgetRectInScreen.maxY() > static_cast<int>(screen.maxY()**))
>> {
>> +            if (widgetRectInScreen.y() - widgetRectInScreen.height() -
>> targetControlHeight > 0) {
>>                 // There is enough room to open upwards.
>> -                widgetRect.move(0, -(widgetRect.height() +
>> targetControlHeight));
>> +                widgetRectInScreen.move(0, -(widgetRectInScreen.height()
>> + targetControlHeight));
>>             } else {
>>                 // Figure whether upwards or downwards has more room and
>> set the
>>                 // maximum number of items.
>> -                int spaceAbove = widgetRect.y() - targetControlHeight;
>> -                int spaceBelow = screen.maxY() - widgetRect.y();
>> +                int spaceAbove = widgetRectInScreen.y() -
>> targetControlHeight;
>> +                int spaceBelow = screen.maxY() - widgetRectInScreen.y();
>>                 if (spaceAbove > spaceBelow)
>>                     m_listBox->setMaxHeight(**spaceAbove);
>>                 else
>> @@ -189,15 +189,16 @@
>>                 // We don't have to recompute X axis, so we only replace
>> Y axis
>>                 // in widgetRect.
>>                 IntRect frameInScreen = chromeClient->**
>> rootViewToScreen(frameRect());
>> -                widgetRect.setY(frameInScreen.**y());
>> -                widgetRect.setHeight(**frameInScreen.height());
>> +                widgetRectInScreen.setY(**frameInScreen.y());
>> +                widgetRectInScreen.setHeight(**frameInScreen.height());
>>                 // And move upwards if necessary.
>>                 if (spaceAbove > spaceBelow)
>> -                    widgetRect.move(0, -(widgetRect.height() +
>> targetControlHeight));
>> +                    widgetRectInScreen.move(0,
>> -(widgetRectInScreen.height() + targetControlHeight));
>>             }
>>         }
>>     }
>> -    return widgetRect;
>> +
>> +    return widgetRectInScreen;
>>  }
>>
>>  void PopupContainer::showPopup(**FrameView* view)
>> @@ -207,7 +208,7 @@
>>
>>     ChromeClientChromium* chromeClient = chromeClientChromium();
>>     if (chromeClient) {
>> -        IntRect popupRect = frameRect();
>> +        IntRect popupRect = m_originalFrameRect;
>>         chromeClient->popupOpened(**this,
layoutAndCalculateWidgetRect(**popupRect.height(),
>> popupRect.location()), false);
>>         m_popupOpen = true;
>>     }
>> @@ -400,29 +401,33 @@
>>     location.move(0, r.height());
>>
>>     m_originalFrameRect = IntRect(location, r.size());
>> -    setFrameRect(m_**originalFrameRect);
>> +
>> +    // Position at (0, 0) since the frameRect().location() is relative
>> to the parent WebWidget.
>> +    setFrameRect(IntRect(IntPoint(**), r.size()));
>>     showPopup(v);
>>  }
>>
>>  void PopupContainer::refresh(const IntRect& targetControlRect)
>>  {
>> -    IntPoint location = m_frameView->contentsToWindow(**
>> targetControlRect.location());
>> +    listBox()->setBaseWidth(max(m_**originalFrameRect.width() -
>> kBorderSize * 2, 0));
>> +    listBox()->updateFromElement()**;
>> +
>> +    IntPoint locationInWindow = m_frameView->contentsToWindow(**
>> targetControlRect.location());
>> +
>>     // Move it below the select widget.
>> -    location.move(0, targetControlRect.height());
>> +    locationInWindow.move(0, targetControlRect.height());
>>
>> -    listBox()->setBaseWidth(max(m_**originalFrameRect.width() -
>> kBorderSize * 2, 0));
>> +    IntRect widgetRectInScreen =
layoutAndCalculateWidgetRect(**targetControlRect.height(),
>> locationInWindow);
>>
>> -    listBox()->updateFromElement()**;
>> -    // Store the original size to check if we need to request the
>> location.
>> -    IntSize originalSize = size();
>> -    IntRect widgetRect =
layoutAndCalculateWidgetRect(**targetControlRect.height(),
>> location);
>> -    if (originalSize != widgetRect.size()) {
>> -        ChromeClientChromium* chromeClient = chromeClientChromium();
>> -        if (chromeClient) {
>> -            IntPoint widgetLocation = chromeClient->**
>> screenToRootView(widgetRect.**location());
>> -            widgetRect.setLocation(**widgetLocation);
>> -            setFrameRect(widgetRect);
>> -        }
>> +    // Reset the size (which can be set to the PopupListBox size in
>> layoutAndGetRTLOffset(), exceeding the available widget rectangle.)
>> +    if (size() != widgetRectInScreen.size())
>> +        resize(widgetRectInScreen.**size());
>> +
>> +    ChromeClientChromium* chromeClient = chromeClientChromium();
>> +    if (chromeClient) {
>> +        // Update the WebWidget location (which is relative to the
>> screen origin).
>> +        if (widgetRectInScreen != chromeClient->windowRect())
>> +            chromeClient->setWindowRect(**widgetRectInScreen);
>>     }
>>
>>     invalidate();
>> Index: Source/WebCore/platform/**chromium/PopupContainer.h
>> ==============================**==============================**=======
>> --- Source/WebCore/platform/**chromium/PopupContainer.h   (revision
>> 115617)
>> +++ Source/WebCore/platform/**chromium/PopupContainer.h   (working copy)
>> @@ -138,7 +138,13 @@
>>
>>     PopupContainerSettings m_settings;
>>     PopupType m_popupType;
>> +
>> +    // This contains the "ideal" dimensions and position for the popup
>> +    // (PopupContainer's frameRect() location should always be (0, 0),
>> since
>> +    // it is rendered inside (and relative to) a WebWidget, which should
>> get
>> +    // the actual popup position through chromeClientChromium()).
>>     IntRect m_originalFrameRect;
>> +
>>     // Whether the popup is currently open.
>>     bool m_popupOpen;
>>  };
>>
>>
>>
>
>
> --
> -alexander
>

Powered by Google App Engine
This is Rietveld 408576698