Summary: | [Chromium] Select popup box does not align with select button when appearance is not "menulist". | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | xiyuan <xiyuan> | ||||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, dglazkov, tony | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Linux | ||||||||||||
Attachments: |
|
Description
xiyuan
2011-02-07 14:26:22 PST
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. |