Bug 113220 – Should close select popup when the element loses focus

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Gustavo Noronha (kov)]

Doing that would make us match Firefox'es behaviour.

[Gustavo Noronha (kov)]

Created attachment 194885 [details]
Patch

[Build Bot]

Comment on attachment 194885 [details]
Patch

Attachment 194885 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17330037

New failing tests:
fast/forms/select-popup-closes-on-blur.html

[Build Bot]

Created attachment 194892 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2

[Gustavo Noronha (kov)]

Created attachment 194913 [details]
Patch

[Kent Tamura]

Comment on attachment 194913 [details]
Patch

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

> Source/WebCore/html/HTMLSelectElement.cpp:1231
> +        if (RenderMenuList* menuList = toRenderMenuList(renderer())) {

We need to check renderer()->isMenuList(). A blur event handler in capturing phase can modify renderer state before the blur event reaches to the target element.

> Source/WebCore/testing/Internals.cpp:2068
> +    HTMLSelectElement* select = toHTMLSelectElement(node);

Possible bad cast.  Please check element type even in testing code not to disturb fuzzers.

> Source/WebCore/testing/Internals.cpp:2069
> +    RenderMenuList* menuList = toRenderMenuList(select->renderer());

ditto.

> LayoutTests/fast/forms/select-popup-closes-on-blur-expected.txt:3
> +PASS internals.isSelectPopupVisible(popup) is true
> +PASS internals.isSelectPopupVisible(popup) is false
> +Test for http://bugs.webkit.org/show_bug.cgi?id=113220 Ensures select popup closes when focus changes.

Test description after test results is not good.  Please use description() to show the description.

[Kent Tamura]

Comment on attachment 194913 [details]
Patch

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

>> Source/WebCore/html/HTMLSelectElement.cpp:1231
>> +        if (RenderMenuList* menuList = toRenderMenuList(renderer())) {
> 
> We need to check renderer()->isMenuList(). A blur event handler in capturing phase can modify renderer state before the blur event reaches to the target element.

Ah, my comment was wrong. We don't need to check it. because usesMenuList in defaultEventHandler ensures it.

[Kent Tamura]

Comment on attachment 194913 [details]
Patch

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

> LayoutTests/ChangeLog:9
> +        * fast/forms/select-popup-closes-on-blur-expected.txt: Added.
> +        * fast/forms/select-popup-closes-on-blur.html: Added.

Please put the test into fast/forms/select/.

[Gustavo Noronha (kov)]

Created attachment 195097 [details]
Patch

[Build Bot]

Comment on attachment 195097 [details]
Patch

Attachment 195097 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17311349

New failing tests:
fast/forms/select/popup-closes-on-blur.html
fast/repaint/japanese-rl-selection-repaint-in-regions.html

[Build Bot]

Created attachment 195107 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2

[Build Bot]

Comment on attachment 195097 [details]
Patch

Attachment 195097 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17335028

[Gustavo Noronha (kov)]

Created attachment 195120 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 195120 [details]
Patch

Attachment 195120 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17172707

[Gustavo Noronha (kov)]

GTK+ EWS failure caused by github being down (it's where it downloads the test fonts from), we should probably make the jhbuild build step a bit more robust.

[Build Bot]

Comment on attachment 195120 [details]
Patch

Attachment 195120 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17147885

[Kent Tamura]

Comment on attachment 195120 [details]
Patch

ok

[Build Bot]

Comment on attachment 195120 [details]
Patch

Attachment 195120 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17295408

New failing tests:
fast/forms/select/popup-closes-on-blur.html

[Build Bot]

Created attachment 195288 [details]
Archive of layout-test-results from webkit-ews-01 for mac-future

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-future  Platform: Mac OS X 10.8.2

[Build Bot]

Comment on attachment 195120 [details]
Patch

Attachment 195120 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17231648

New failing tests:
fast/forms/select/popup-closes-on-blur.html

[Build Bot]

Created attachment 195294 [details]
Archive of layout-test-results from webkit-ews-05 for mac-future

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-future  Platform: Mac OS X 10.8.2

[Gustavo Noronha (kov)]

Created attachment 195316 [details]
Patch

Patch for landing, final run through the EWS

[Build Bot]

Comment on attachment 195316 [details]
Patch

Attachment 195316 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17314430

[Build Bot]

Comment on attachment 195316 [details]
Patch

Attachment 195316 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17251709

New failing tests:
fast/forms/select/popup-closes-on-blur.html

[Build Bot]

Created attachment 195507 [details]
Archive of layout-test-results from webkit-ews-01 for mac-future

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-future  Platform: Mac OS X 10.8.2

[Gustavo Noronha (kov)]

Created attachment 196223 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 196223 [details]
Patch

Clearing flags on attachment: 196223

Committed r147548: <http://trac.webkit.org/changeset/147548>

[Gustavo Noronha (kov)]

All reviewed patches have been landed.  Closing bug.