resizeTo/resizeBy causes rendering problems in <select> dropdowns
Reported by
mdicke...@gmail.com,
Aug 17 2016
|
|||||||||||||||||||
Issue description
Chrome Version : 52.0.2743.116 m
URLs (if applicable) : <N/A>
Other browsers tested:
Chromium: FAIL 52.0.2743.0
Chromium: PASS 51.0.2704.0
Firefox: PASS 48
IE: PASS 11
What steps will reproduce the problem?
(1) Open the attached resizebug.html
(2) Optionally, press "Popup" to open the page as a popup, the bug occurs in either case.
(3) Immediately after the page loads, try opening one of the dropdowns.
(4) It will not occur again until the page is refreshed
What is the expected result?
The dropdown appears directly below or directly above the select element.
What happens instead?
In the unstyled case:
The dropdown appears offset from where it is supposed to appear.
When a transition style is applied to the select element:
The dropdown contents is offset by black space. Some options are clipped and can't be clicked.
Additional information:
This only occurs when the resizeTo or resizeBy results in the window staying the same size.
This can happen in these cases:
- resize function was not called from a popup, so resizing isn't allowed.
- a popup was already at the size passed to resizeTo
- calling resizeBy(0,0)
The dropdowns return to normal after opening one, manually resizing the window, or moving the window.
I have only seen the black space happen using the transition style, but it might happen with others.
,
Aug 18 2016
,
Aug 19 2016
Actually, on closer inspection this is actually a separate bug.
,
Aug 24 2016
,
Aug 26 2016
I found a different way of causing the black box bug to happen.
1. Open the resizebug2.html attached to this comment.
2. Position the window so that the drop down would have to expand upwards.
3. Click the drop down
There is a click event attached to the select element which adds options to the list when it is clicked. In the case I observed this, the select list was being dynamically populated with the result of an AJAX request.
This is obviously bad practice, but it demonstrates the issue in a different way which hopefully is useful.
Browsers tested:
Chromium: FAIL 52.0.2743.0
Chromium: PASS 51.0.2704.0
Firefox: PASS 48
IE: PASS 11
,
Sep 7 2016
Issue 642349 has been merged into this issue.
,
Sep 9 2016
,
Sep 12 2016
Issue 645863 has been merged into this issue.
,
Sep 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f8386d1d8f1268ef794fafef76d7dd5b372367a9 commit f8386d1d8f1268ef794fafef76d7dd5b372367a9 Author: bokan <bokan@chromium.org> Date: Fri Sep 23 03:03:47 2016 Remove window.moveTo calls from picker-common.js These moveTo calls appear to be unneeded and they don't work on the main window anyway. Due to the way that RenderWidget caches move requests, this causes us to return incorrect values for the windowRect until the browser resets the rect. This appears to work correctly today due to a bug in how the pending_window_rect gets cached in RenderWidget but makes it difficult to fix that bug. See related CL: https://codereview.chromium.org/2333353002/ Note: This change affects only LayoutTests. BUG= 638671 Review-Url: https://codereview.chromium.org/2354273002 Cr-Commit-Position: refs/heads/master@{#420557} [modify] https://crrev.com/f8386d1d8f1268ef794fafef76d7dd5b372367a9/third_party/WebKit/LayoutTests/fast/forms/resources/picker-common.js
,
Oct 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c commit 6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c Author: bokan <bokan@chromium.org> Date: Wed Oct 05 00:55:21 2016 Don't set view_screen_rect_ on RequestMove ACK In https://codereview.chromium.org/1894333002/ I made the OnRequestMoveAck method reset the view_screen_rect_ variable to the pending rect. This was a mistake because the move request is on the *window* rect, not the view rect. Additionally, it's unnecessary since it's the browser's job to send an UpdateScreenRects message to the renderer with the new view and window bounds. This only happens for non-popup windows so popups will now store the view/window rects immediately. This patch removes the reset in the Ack method. In addition, the windowRect method confusingly returns the view_screen_rect or pending_window_rect if there's a pending move. I've changed this to return the window_screen_rect_ and added a viewRect method for callers that actually expect the view. I also did some cleanup and replaced the redundant RootWindowRect with windowRect. BUG= 638671 Review-Url: https://codereview.chromium.org/2333353002 Cr-Commit-Position: refs/heads/master@{#423030} [modify] https://crrev.com/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c/content/renderer/gpu/gpu_benchmarking_extension.cc [modify] https://crrev.com/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c/content/renderer/render_view_impl.cc [modify] https://crrev.com/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c/content/renderer/render_widget.cc [modify] https://crrev.com/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c/content/renderer/render_widget.h [modify] https://crrev.com/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c/third_party/WebKit/LayoutTests/fast/forms/select/input-select-after-resize-expected.txt [add] https://crrev.com/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c/third_party/WebKit/LayoutTests/fast/forms/select/input-select-after-resize.html [add] https://crrev.com/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c/third_party/WebKit/LayoutTests/platform/linux/fast/forms/select/input-select-after-resize-expected.png [modify] https://crrev.com/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c/third_party/WebKit/Source/web/ChromeClientImpl.cpp [modify] https://crrev.com/6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c/third_party/WebKit/public/web/WebWidgetClient.h
,
Oct 6 2016
,
Oct 6 2016
Confirmed fix in Canary. Requesting merge to M54. Note, this is related and required to merge the fix in issue 653327 .
,
Oct 6 2016
Your change meets the bar and is auto-approved for M54 (branch: 2840)
,
Oct 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7941224be415b3ba40c1e1e536b7545b215e4560 commit 7941224be415b3ba40c1e1e536b7545b215e4560 Author: David Bokan <bokan@chromium.org> Date: Thu Oct 06 13:47:41 2016 Don't set view_screen_rect_ on RequestMove ACK In https://codereview.chromium.org/1894333002/ I made the OnRequestMoveAck method reset the view_screen_rect_ variable to the pending rect. This was a mistake because the move request is on the *window* rect, not the view rect. Additionally, it's unnecessary since it's the browser's job to send an UpdateScreenRects message to the renderer with the new view and window bounds. This only happens for non-popup windows so popups will now store the view/window rects immediately. This patch removes the reset in the Ack method. In addition, the windowRect method confusingly returns the view_screen_rect or pending_window_rect if there's a pending move. I've changed this to return the window_screen_rect_ and added a viewRect method for callers that actually expect the view. I also did some cleanup and replaced the redundant RootWindowRect with windowRect. BUG= 638671 Review-Url: https://codereview.chromium.org/2333353002 Cr-Commit-Position: refs/heads/master@{#423030} (cherry picked from commit 6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c) Review URL: https://codereview.chromium.org/2398763003 . Cr-Commit-Position: refs/branch-heads/2840@{#662} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/content/renderer/gpu/gpu_benchmarking_extension.cc [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/content/renderer/render_view_impl.cc [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/content/renderer/render_widget.cc [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/content/renderer/render_widget.h [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/LayoutTests/fast/forms/select/input-select-after-resize-expected.txt [add] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/LayoutTests/fast/forms/select/input-select-after-resize.html [add] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/LayoutTests/platform/linux/fast/forms/select/input-select-after-resize-expected.png [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/Source/web/ChromeClientImpl.cpp [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/public/web/WebWidgetClient.h
,
Oct 7 2016
,
Oct 12 2016
Tested this issue on Windows-10 and Ubuntu 14.04 using chrome latest Beta M54-54.0.2840.59 by following steps mentioned in the original comment. Observed drop down appears directly below the select element as expected. Hence adding TE-Verified label.
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7941224be415b3ba40c1e1e536b7545b215e4560 commit 7941224be415b3ba40c1e1e536b7545b215e4560 Author: David Bokan <bokan@chromium.org> Date: Thu Oct 06 13:47:41 2016 Don't set view_screen_rect_ on RequestMove ACK In https://codereview.chromium.org/1894333002/ I made the OnRequestMoveAck method reset the view_screen_rect_ variable to the pending rect. This was a mistake because the move request is on the *window* rect, not the view rect. Additionally, it's unnecessary since it's the browser's job to send an UpdateScreenRects message to the renderer with the new view and window bounds. This only happens for non-popup windows so popups will now store the view/window rects immediately. This patch removes the reset in the Ack method. In addition, the windowRect method confusingly returns the view_screen_rect or pending_window_rect if there's a pending move. I've changed this to return the window_screen_rect_ and added a viewRect method for callers that actually expect the view. I also did some cleanup and replaced the redundant RootWindowRect with windowRect. BUG= 638671 Review-Url: https://codereview.chromium.org/2333353002 Cr-Commit-Position: refs/heads/master@{#423030} (cherry picked from commit 6b08cd23a5bbe4e1d95bb7e0431fc743e6b0180c) Review URL: https://codereview.chromium.org/2398763003 . Cr-Commit-Position: refs/branch-heads/2840@{#662} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/content/renderer/gpu/gpu_benchmarking_extension.cc [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/content/renderer/render_view_impl.cc [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/content/renderer/render_widget.cc [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/content/renderer/render_widget.h [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/LayoutTests/fast/forms/select/input-select-after-resize-expected.txt [add] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/LayoutTests/fast/forms/select/input-select-after-resize.html [add] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/LayoutTests/platform/linux/fast/forms/select/input-select-after-resize-expected.png [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/Source/web/ChromeClientImpl.cpp [modify] https://crrev.com/7941224be415b3ba40c1e1e536b7545b215e4560/third_party/WebKit/public/web/WebWidgetClient.h |
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by durga.behera@chromium.org
, Aug 18 2016Components: Blink>Forms>Select
Labels: M-54 OS-Linux OS-Windows
Owner: bokan@chromium.org
Status: Assigned (was: Unconfirmed)