Modified: trunk/Source/WebCore/ChangeLog (125748 => 125749)
--- trunk/Source/WebCore/ChangeLog 2012-08-16 05:59:39 UTC (rev 125748)
+++ trunk/Source/WebCore/ChangeLog 2012-08-16 06:01:33 UTC (rev 125749)
@@ -1,3 +1,35 @@
+2012-08-15 Kent Tamura <[email protected]>
+
+ [Chromium] Refactoring: Introduce a new function for some part of PopupContainer::layoutAndCalculateWidgetRect()
+ https://bugs.webkit.org/show_bug.cgi?id=94087
+
+ Reviewed by Hajime Morita.
+
+ Move some part of PopupContainer::layoutAndCalculateWidgetRect() to a
+ new function which is not a member of PopupContainer because we'd like
+ to add a unit test for the position calculation code, and to reduce the
+ dependency.
+
+ No new tests. Popup positioning code is not testable in WebKit.
+
+ * platform/chromium/PopupContainer.cpp:
+ (WebCore::layoutAndCalculateWidgetRectInternal):
+ Added. Move the code from PopupContainer::layoutAndCalculateWidgetRect.
+ In order to avoid to call member functions of PopupContainer, we
+ don't call layoutAndGetRTLOffset() and height(). Use
+ PopupListBox::layout() to recalculate the popup content size, and use
+ PopupListBox::height() + kBorderSize * 2 instead of height(). We
+ resize the view after finishing layoutAndCalculateWidgetRectInternal
+ in PopupContainer::layoutAndCalculateWidgetRect.
+ (WebCore::PopupContainer::layoutAndCalculateWidgetRect):
+ Move some code to layoutAndCalculateWidgetRectInternal.
+ (WebCore::PopupContainer::fitToListBox):
+ Added. Move the code from PopupContainer::layoutAndGetRTLOffset.
+ (WebCore::PopupContainer::layoutAndGetRTLOffset):
+ Move some code to fitToListBox.
+ * platform/chromium/PopupContainer.h:
+ (PopupContainer): Added fitToListBox.
+
2012-08-15 Adam Barth <[email protected]>
VoidCallback should not be a special snowflake
Modified: trunk/Source/WebCore/platform/chromium/PopupContainer.cpp (125748 => 125749)
--- trunk/Source/WebCore/platform/chromium/PopupContainer.cpp 2012-08-16 05:59:39 UTC (rev 125748)
+++ trunk/Source/WebCore/platform/chromium/PopupContainer.cpp 2012-08-16 06:01:33 UTC (rev 125749)
@@ -117,6 +117,59 @@
removeChild(m_listBox.get());
}
+static IntRect layoutAndCalculateWidgetRectInternal(IntRect widgetRectInScreen, int targetControlHeight, const FloatRect& windowRect, const FloatRect& screen, bool isRTL, const int rtlOffset, PopupListBox* listBox, bool& needToResizeView)
+{
+ ASSERT(listBox);
+ 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 inverseWidgetRectInScreen = widgetRectInScreen;
+ inverseWidgetRectInScreen.setX(inverseWidgetRectInScreen.x() + (isRTL ? -rtlOffset : rtlOffset));
+ IntRect enclosingScreen = enclosingIntRect(screen);
+ unsigned originalCutoff = std::max(enclosingScreen.x() - widgetRectInScreen.x(), 0) + std::max(widgetRectInScreen.maxX() - enclosingScreen.maxX(), 0);
+ unsigned inverseCutoff = std::max(enclosingScreen.x() - inverseWidgetRectInScreen.x(), 0) + std::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)
+ widgetRectInScreen = inverseWidgetRectInScreen;
+
+ if (widgetRectInScreen.x() < screen.x()) {
+ unsigned widgetRight = widgetRectInScreen.maxX();
+ widgetRectInScreen.setWidth(widgetRectInScreen.maxX() - screen.x());
+ widgetRectInScreen.setX(widgetRight - widgetRectInScreen.width());
+ listBox->setMaxWidthAndLayout(std::max(widgetRectInScreen.width() - kBorderSize * 2, 0));
+ } else if (widgetRectInScreen.maxX() > screen.maxX()) {
+ widgetRectInScreen.setWidth(screen.maxX() - widgetRectInScreen.x());
+ listBox->setMaxWidthAndLayout(std::max(widgetRectInScreen.width() - kBorderSize * 2, 0));
+ }
+ }
+
+ // Calculate Y axis size.
+ if (widgetRectInScreen.maxY() > static_cast<int>(screen.maxY())) {
+ if (widgetRectInScreen.y() - widgetRectInScreen.height() - targetControlHeight > 0) {
+ // There is enough room to open upwards.
+ widgetRectInScreen.move(0, -(widgetRectInScreen.height() + targetControlHeight));
+ } else {
+ // Figure whether upwards or downwards has more room and set the
+ // maximum number of items.
+ int spaceAbove = widgetRectInScreen.y() - targetControlHeight;
+ int spaceBelow = screen.maxY() - widgetRectInScreen.y();
+ if (spaceAbove > spaceBelow)
+ listBox->setMaxHeight(spaceAbove);
+ else
+ listBox->setMaxHeight(spaceBelow);
+ listBox->layout();
+ needToResizeView = true;
+ widgetRectInScreen.setHeight(listBox->height() + kBorderSize * 2);
+ // Move WebWidget upwards if necessary.
+ if (spaceAbove > spaceBelow)
+ widgetRectInScreen.move(0, -(widgetRectInScreen.height() + targetControlHeight));
+ }
+ }
+ return widgetRectInScreen;
+}
+
IntRect PopupContainer::layoutAndCalculateWidgetRect(int targetControlHeight, const IntPoint& popupInitialCoordinate)
{
// Reset the max width and height to their default values, they will be recomputed below
@@ -148,51 +201,11 @@
// 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() && (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 inverseWidgetRectInScreen = chromeClient->rootViewToScreen(IntRect(popupInitialCoordinate.x() + (isRTL ? 0 : rtlOffset), popupInitialCoordinate.y(), targetSize.width(), targetSize.height()));
- IntRect enclosingScreen = enclosingIntRect(screen);
- 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)
- widgetRectInScreen = inverseWidgetRectInScreen;
-
- 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 (widgetRectInScreen.maxY() > static_cast<int>(screen.maxY())) {
- if (widgetRectInScreen.y() - widgetRectInScreen.height() - targetControlHeight > 0) {
- // There is enough room to open upwards.
- widgetRectInScreen.move(0, -(widgetRectInScreen.height() + targetControlHeight));
- } else {
- // Figure whether upwards or downwards has more room and set the
- // maximum number of items.
- int spaceAbove = widgetRectInScreen.y() - targetControlHeight;
- int spaceBelow = screen.maxY() - widgetRectInScreen.y();
- if (spaceAbove > spaceBelow)
- m_listBox->setMaxHeight(spaceAbove);
- else
- m_listBox->setMaxHeight(spaceBelow);
- layoutAndGetRTLOffset();
- // Container height may have changed in layoutAndGetRTLOffset(),
- // so set the WebWidget height to the container height.
- widgetRectInScreen.setHeight(height());
- // Move WebWidget upwards if necessary.
- if (spaceAbove > spaceBelow)
- widgetRectInScreen.move(0, -(widgetRectInScreen.height() + targetControlHeight));
- }
- }
+ bool needToResizeView = false;
+ widgetRectInScreen = layoutAndCalculateWidgetRectInternal(widgetRectInScreen, targetControlHeight, windowRect, screen, isRTL, rtlOffset, m_listBox.get(), needToResizeView);
+ if (needToResizeView)
+ fitToListBox();
}
return widgetRectInScreen;
@@ -235,23 +248,26 @@
chromeClientChromium()->popupClosed(this);
}
-int PopupContainer::layoutAndGetRTLOffset()
+void PopupContainer::fitToListBox()
{
- m_listBox->layout();
-
// Place the listbox within our border.
m_listBox->move(kBorderSize, kBorderSize);
// Size ourselves to contain listbox + border.
- int listBoxWidth = m_listBox->width() + kBorderSize * 2;
- resize(listBoxWidth, m_listBox->height() + kBorderSize * 2);
+ resize(m_listBox->width() + kBorderSize * 2, m_listBox->height() + kBorderSize * 2);
invalidate();
+}
+int PopupContainer::layoutAndGetRTLOffset()
+{
+ m_listBox->layout();
+ fitToListBox();
+
// Compute the starting x-axis for a normal RTL or right-aligned LTR dropdown. For those,
// the right edge of dropdown box should be aligned with the right edge of <select>/<input> element box,
// and the dropdown box should be expanded to the left if more space is needed.
// m_originalFrameRect.width() is the width of the target <select>/<input> element.
- return m_originalFrameRect.width() - listBoxWidth;
+ return m_originalFrameRect.width() - (m_listBox->width() + kBorderSize * 2);
}
bool PopupContainer::handleMouseDownEvent(const PlatformMouseEvent& event)
Modified: trunk/Source/WebCore/platform/chromium/PopupContainer.h (125748 => 125749)
--- trunk/Source/WebCore/platform/chromium/PopupContainer.h 2012-08-16 05:59:39 UTC (rev 125748)
+++ trunk/Source/WebCore/platform/chromium/PopupContainer.h 2012-08-16 06:01:33 UTC (rev 125749)
@@ -130,6 +130,8 @@
// Layout and calculate popup widget size and location and returns it as IntRect.
IntRect layoutAndCalculateWidgetRect(int targetControlHeight, const IntPoint& popupInitialCoordinate);
+ void fitToListBox();
+
// Returns the ChromeClient of the page this popup is associated with.
ChromeClientChromium* chromeClientChromium();
