MD WebUI: Fix incorrect positioning of action menu after window resize

MD WebUI: Fix incorrect positioning of action menu after window resize

Showing the menu, resizing the browser window horizontally and then
showing the menu a second time could result in incorrectly positioning
the window, as position values from the first instance would affect
layout of the second instance. This fixes the issue by clearing position
alues before reshowing the menu.

BUG=667198
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2614663011
Cr-Commit-Position: refs/heads/master@{#443429}
Committed: https://chromium.googlesource.com/chromium/src/+/b1caf20ec92bdf673b7b86adfca7e5121914551a

[tsergeant, 2017-01-06 05:18:46]

Description was changed from

==========
MD WebUI: Fix incorrect positioning of action menu after window resize

Showing the menu, resizing the browser window horizontally and then
showing the menu a second time could result in incorrectly positioning
the window, as position values from the first instance would affect
layout of the second instance. This fixes the issue by clearing position
alues before reshowing the menu.

BUG=667198
==========

to

==========
MD WebUI: Fix incorrect positioning of action menu after window resize

Showing the menu, resizing the browser window horizontally and then
showing the menu a second time could result in incorrectly positioning
the window, as position values from the first instance would affect
layout of the second instance. This fixes the issue by clearing position
alues before reshowing the menu.

BUG=667198
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2017-01-06 05:21:10]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 05:21:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tsergeant, 2017-01-06 05:23:07]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 05:23:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tsergeant, 2017-01-06 05:23:52]

tsergeant@chromium.org changed reviewers:
+ dpapad@chromium.org

[tsergeant, 2017-01-06 05:23:53]

PTAL!

I couldn't think of anyway to write a regression test for this -- let me know if
you have any ideas.

[commit-bot: I haz the power, 2017-01-06 06:19:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 06:19:02]

Dry run: This issue passed the CQ dry run.

[dpapad, 2017-01-06 16:34:49]

https://codereview.chromium.org/2614663011/diff/20001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js (right):

https://codereview.chromium.org/2614663011/diff/20001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js:140: var rect =
this.anchorElement_.getBoundingClientRect();
I don't fully understand this fix. Per my own debugging at
https://bugs.chromium.org/p/chromium/issues/detail?id=667198#c2 (see video), the
getBoundingClientRect() call here returns the wrong values (clearly shown in the
video, unless I am mistaken). How is this affected by the previous values of
this.style.left/right/top? 

Are you 100% sure that this fixes the problem?

[dpapad, 2017-01-12 18:52:54]

LGTM

https://codereview.chromium.org/2614663011/diff/20001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js (right):

https://codereview.chromium.org/2614663011/diff/20001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js:140: var rect =
this.anchorElement_.getBoundingClientRect();
On 2017/01/06 at 16:34:49, dpapad wrote:
> I don't fully understand this fix. Per my own debugging at
https://bugs.chromium.org/p/chromium/issues/detail?id=667198#c2 (see video), the
getBoundingClientRect() call here returns the wrong values (clearly shown in the
video, unless I am mistaken). How is this affected by the previous values of
this.style.left/right/top? 
> 
> Are you 100% sure that this fixes the problem?

Per explanation at
https://bugs.chromium.org/p/chromium/issues/detail?id=667198#c13, I know
understand how this fixes the issue. Thanks.

[tsergeant, 2017-01-12 22:57:20]

The CQ bit was checked by tsergeant@chromium.org

[commit-bot: I haz the power, 2017-01-12 22:58:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 01:20:23]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484261840600500,
"parent_rev": "af0d4819e446c6615c38b018c58d82565b20716d", "commit_rev":
"b1caf20ec92bdf673b7b86adfca7e5121914551a"}

[commit-bot: I haz the power, 2017-01-13 01:21:32]

Description was changed from

==========
MD WebUI: Fix incorrect positioning of action menu after window resize

Showing the menu, resizing the browser window horizontally and then
showing the menu a second time could result in incorrectly positioning
the window, as position values from the first instance would affect
layout of the second instance. This fixes the issue by clearing position
alues before reshowing the menu.

BUG=667198
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD WebUI: Fix incorrect positioning of action menu after window resize

Showing the menu, resizing the browser window horizontally and then
showing the menu a second time could result in incorrectly positioning
the window, as position values from the first instance would affect
layout of the second instance. This fixes the issue by clearing position
alues before reshowing the menu.

BUG=667198
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2614663011
Cr-Commit-Position: refs/heads/master@{#443429}
Committed:
https://chromium.googlesource.com/chromium/src/+/b1caf20ec92bdf673b7b86adfca7...
==========

[commit-bot: I haz the power, 2017-01-13 01:21:33]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/b1caf20ec92bdf673b7b86adfca7...