Bug 37232 – [Chromium] RTL <select> dropdown box expands to right instead of left

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Xiaomei Ji]

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.

[Xiaomei Ji]

Created attachment 52783 [details]
expected behavior

[Xiaomei Ji]

related chromium bug:
http://code.google.com/p/chromium/issues/detail?id=40582

[Xiaomei Ji]

Created attachment 52994 [details]
patch

[Xiaomei Ji]

Jay reviewed the patch in chromium's codereview site:
http://codereview.chromium.org/1621011/show

[Dimitri Glazkov (Google)]

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();
>  }

[Xiaomei Ji]

Created attachment 53899 [details]
patch w/ manual test

[Xiaomei Ji]

(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();
> >  }

[Dimitri Glazkov (Google)]

Comment on attachment 53899 [details]
patch w/ manual test

ok.

[Xiaomei Ji]

Created attachment 53903 [details]
patch w/ manual test

[Dimitri Glazkov (Google)]

Comment on attachment 53903 [details]
patch w/ manual test

per author's request.

[Xiaomei Ji]

Manually committed r57992: http://trac.webkit.org/changeset/57992