Created attachment 81520 [details] Test case. When a select element's appearance is not "menulist", we use the css padding to calculdate popup width (since https://bugs.webkit.org/show_bug.cgi?id=33235). And this caused the popup box is wider than the select button when appearance is not menulist. And use has no way to fix this via css padding.
Created attachment 81523 [details] Proposed patch.
Created attachment 81525 [details] Fix typo in manual test name.
Comment on attachment 81525 [details] Fix typo in manual test name. View in context: https://bugs.webkit.org/attachment.cgi?id=81525&action=review just some small nits > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1272 > +const int minEndOfLinePadding = 2; static? Also, should this be at the top of the file with the other constants? The other constants are named incorrectly (k prefix), but we can fix that later. > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1275 > + bool rightAligned = m_popupClient->menuStyle().textDirection() == RTL; Nit: rightAligned -> isRightAligned
Comment on attachment 81525 [details] Fix typo in manual test name. View in context: https://bugs.webkit.org/attachment.cgi?id=81525&action=review > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1349 > + // Use minEndOfLinePadding when there is a scrollbar. Why does a scrollbar cause us to need 2 extra pixels?
Comment on attachment 81525 [details] Fix typo in manual test name. View in context: https://bugs.webkit.org/attachment.cgi?id=81525&action=review >> Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1272 > > static? Also, should this be at the top of the file with the other constants? The other constants are named incorrectly (k prefix), but we can fix that later. Done. >> Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1275 > > Nit: rightAligned -> isRightAligned Done >> Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1349 >> + // Use minEndOfLinePadding when there is a scrollbar. > > Why does a scrollbar cause us to need 2 extra pixels? Updated the comment to make it more clear. It's not that we need the 2 extra pixels. We replace the lineEndPaddingWidth space with the minimum 2 pixel space when there is a scrollbar. In this way, scrollbar will stay as much as possible inside the line end padding space. User could use a big enough "padding-right" (>= scrollbarWidth + 2px) to make the popup listbox align with the select button. This is because https://bugs.webkit.org/show_bug.cgi?id=33235 makes popup respect the "padding-right". It works well when there is no scrollbar. But when popup has a scrollbar, the popup is always wider than the select button no matter what padding-right value you use. You can see that from the test case page.
Created attachment 81550 [details] Address tony's comments @3,4
Comment on attachment 81550 [details] Address tony's comments @3,4 View in context: https://bugs.webkit.org/attachment.cgi?id=81550&action=review > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1353 > + // Use kMinEndOfLinePadding when there is a scrollbar so that we use > + // as much as (lineEndPaddingWidth - kMinEndOfLinePadding) padding > + // space for scrollbar and allow user to use CSS padding to make the > + // popup listbox align with the select element. > + paddingWidth = paddingWidth - lineEndPaddingWidth + kMinEndOfLinePadding; This comment is much more useful. Thanks!
Comment on attachment 81550 [details] Address tony's comments @3,4 Clearing flags on attachment: 81550 Committed r77907: <http://trac.webkit.org/changeset/77907>
All reviewed patches have been landed. Closing bug.