Bug 53632

Summary: [Chromium] Option text in select popup does not align with menulist button text
Product: WebKit Reporter: xiyuan <xiyuan>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dglazkov, sam, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Screenshot of problem.
none
Proposed patch. none

Description xiyuan 2011-02-02 15:24:03 PST
Created attachment 80978 [details]
Screenshot of problem.

Menulist button has addition paddings from m_innerBlock besides css paddings and PopupListBox should include those paddings as well.
Comment 1 xiyuan 2011-02-02 15:54:52 PST
Created attachment 80987 [details]
Proposed patch.
Comment 2 xiyuan 2011-02-02 16:18:44 PST
Comment on attachment 80987 [details]
Proposed patch.

This patch has side effect that because the m_innerBlock padding include the space for scrollbar and PopupListBox::layout might add space for scrollbar again.
Comment 3 xiyuan 2011-02-02 16:50:43 PST
Comment on attachment 80987 [details]
Proposed patch.

Resurrect the patch as we are protected from counting scrollbar width twice in RenderMenuList::clientPaddingRight which returns fixed endOfLinePadding.
Comment 4 Tony Chang 2011-02-04 09:51:57 PST
Comment on attachment 80987 [details]
Proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=80987&action=review

> Source/WebCore/rendering/RenderMenuList.cpp:489
> +    return paddingLeft() + m_innerBlock->paddingLeft();

Won't this change the offset for all webkit platforms?  Is that intentional?
Comment 5 xiyuan 2011-02-04 10:32:13 PST
(In reply to comment #4)
> (From update of attachment 80987 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80987&action=review
> 
> > Source/WebCore/rendering/RenderMenuList.cpp:489
> > +    return paddingLeft() + m_innerBlock->paddingLeft();
> 
> Won't this change the offset for all webkit platforms?  Is that intentional?

This should affect Windows and Linux. Both platforms have the alignment problem.

Mac probably will not be affected by this because its popup shows up via showExternal which should use native widget for the popup.
Comment 6 Tony Chang 2011-02-04 11:05:18 PST
Comment on attachment 80987 [details]
Proposed patch.

If it only affects Windows and Linux, this is fine.

For reference, IE & Firefox Win aligns the text in popups.  This is consistent with native menu lists on Windows. GTK+ menu lists also align the text (actually, GTK+ drop downs are like OSX menu lists where the text doesn't move but the options appear above/below the currently selected item).
Comment 7 WebKit Commit Bot 2011-02-04 18:19:56 PST
Comment on attachment 80987 [details]
Proposed patch.

Clearing flags on attachment: 80987

Committed r77716: <http://trac.webkit.org/changeset/77716>
Comment 8 WebKit Commit Bot 2011-02-04 18:20:01 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 WebKit Commit Bot 2011-02-04 19:29:07 PST
The commit-queue encountered the following flaky tests while processing attachment 80987 [details]:

inspector/elements-panel-xhtml-structure.xhtml bug 53835 (authors: apavlov@chromium.org and pfeldman@chromium.org)
The commit-queue is continuing to process your patch.