Bug 37232

Summary: [Chromium] RTL <select> dropdown box expands to right instead of left
Product: WebKit Reporter: Xiaomei Ji <xji>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cira, jshin, playmobil, vivianz, xji
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows Vista   
URL: http://news.google.com/news?ned=ar_me
Attachments:
Description Flags
expected behavior
none
patch
none
patch w/ manual test
dglazkov: review+, dglazkov: commit-queue+
patch w/ manual test dglazkov: review+, dglazkov: commit-queue-

Description Xiaomei Ji 2010-04-07 14:23:40 PDT
Click the <select> box on top-right of http://news.google.com/news?ned=ar_me,
and you will see the dropdown box is left-aligned with the <select> and expand to right if needed.
The dropdown box will expand out of browser window, and it is got cutoff when the browser window is maximized.

For RTL, the dropdown box should be right-aligned with <select> and expand to left if needed, which is the behavior of FireFox in windows.
Comment 1 Xiaomei Ji 2010-04-07 14:32:01 PDT
Created attachment 52783 [details]
expected behavior
Comment 2 Xiaomei Ji 2010-04-07 14:34:06 PDT
related chromium bug:
http://code.google.com/p/chromium/issues/detail?id=40582
Comment 3 Xiaomei Ji 2010-04-09 14:58:57 PDT
Created attachment 52994 [details]
patch
Comment 4 Xiaomei Ji 2010-04-09 15:01:27 PDT
Jay reviewed the patch in chromium's codereview site:
http://codereview.chromium.org/1621011/show
Comment 5 Dimitri Glazkov (Google) 2010-04-20 10:39:09 PDT
Comment on attachment 52994 [details]
patch

>
> +    // Adjust the starting x-axis for RTL dropdown. For RTL dropdown, the right edge
> +    // of dropdown box should be aligned with the right edge of <select> element box,
> +    // and the dropdown box should be expanded to left if more space needed.
> +    bool rightAligned = m_listBox->m_popupClient->menuStyle().textDirection() == RTL;

Can m_popupClient be 0 here?

> +    if (rightAligned)
> +        move(x() + popupWidth - listBoxWidth, y());
>  
>      invalidate();
>  }
Comment 6 Xiaomei Ji 2010-04-20 15:47:41 PDT
Created attachment 53899 [details]
patch w/ manual test
Comment 7 Xiaomei Ji 2010-04-20 15:48:16 PDT
(In reply to comment #5)
> (From update of attachment 52994 [details])
> >
> > +    // Adjust the starting x-axis for RTL dropdown. For RTL dropdown, the right edge
> > +    // of dropdown box should be aligned with the right edge of <select> element box,
> > +    // and the dropdown box should be expanded to left if more space needed.
> > +    bool rightAligned = m_listBox->m_popupClient->menuStyle().textDirection() == RTL;
> 
> Can m_popupClient be 0 here?

I do not think it can be 0 here. But I added a NULL check for safe.

> 
> > +    if (rightAligned)
> > +        move(x() + popupWidth - listBoxWidth, y());
> >  
> >      invalidate();
> >  }
Comment 8 Dimitri Glazkov (Google) 2010-04-20 15:58:27 PDT
Comment on attachment 53899 [details]
patch w/ manual test

ok.
Comment 9 Xiaomei Ji 2010-04-20 16:02:31 PDT
Created attachment 53903 [details]
patch w/ manual test
Comment 10 Dimitri Glazkov (Google) 2010-04-20 16:22:23 PDT
Comment on attachment 53903 [details]
patch w/ manual test

per author's request.
Comment 11 Xiaomei Ji 2010-04-21 10:36:07 PDT
Manually committed r57992: http://trac.webkit.org/changeset/57992