CrElements: Fix Polymer 2 interactive_ui_tests failures |
||||
Issue descriptionThe following tests fail when run with --enable-features=WebUIPolymer2: Only with optimize_webui=true: CrElementsActionMenuTest.All CrElementsInputTest.All Flaky on debug builds (will likely need to repeat tests in order to observe failures): CrElementsCheckboxTest.All CrElementsProfileAvatarSelectorFocusTest.All CrElementsToggleTest.All
,
Sep 5
Candidate CL for CrElementsActionMenuTest.All is at https://chromium-review.googlesource.com/c/chromium/src/+/1208454, although it is not very clear yet what the difference between Shadow DOM v0 and v1 is (which is causing the failure).
,
Sep 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fccfd62a8f0fd29eb9142d6984b97ac952d42067 commit fccfd62a8f0fd29eb9142d6984b97ac952d42067 Author: dpapad <dpapad@chromium.org> Date: Wed Sep 05 23:09:30 2018 WebUI: Make CrElementsActionMenuTest.All pass in Polymer 2. The semantics of document.activeElement and someElement.shadowRoot.activeElement seem to have changed between Shadow DOM v0 and v1. In V1 the following is possible: someElement.shadowRoot.activeElement is null AND document.activeElement points to a child belonging to |someElement|. which does not seem to be the case for Shadow DOM v0. Always recursing from document.activeElement seems more robust and works for both cases. Bug: 875470 Change-Id: I53044c23bcfcfa85264b5154e635e44c05ce0f6f Reviewed-on: https://chromium-review.googlesource.com/1208454 Reviewed-by: Rebekah Potter <rbpotter@chromium.org> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#589049} [modify] https://crrev.com/fccfd62a8f0fd29eb9142d6984b97ac952d42067/chrome/test/data/webui/cr_elements/cr_action_menu_test.js [modify] https://crrev.com/fccfd62a8f0fd29eb9142d6984b97ac952d42067/testing/buildbot/filters/webui_polymer2_interactive_ui_tests.filter [modify] https://crrev.com/fccfd62a8f0fd29eb9142d6984b97ac952d42067/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js [modify] https://crrev.com/fccfd62a8f0fd29eb9142d6984b97ac952d42067/ui/webui/resources/js/util.js
,
Sep 7
FYI started looking at CrElementsInputTest.All. The failure indicates an actual problem with the prod code (notice that focusing a cr-input with the mouse does not show the blue underline in Polymer 2). Somehow the listener at [1] never executes. [1] https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_input/cr_input.js?l=153
,
Sep 7
The ability to add lisetners used |elementId.eventName| syntax as done in [1] has been removed according to [2]. This explains the failure, and we should probably audit all other elements for similar pattern. [1] https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_input/cr_input.js?l=153-156 [2] https://www.polymer-project.org/2.0/docs/upgrade#removed-apis
,
Sep 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/142d451318f046a97c9df1c2459226f1f0e95afd commit 142d451318f046a97c9df1c2459226f1f0e95afd Author: dpapad <dpapad@chromium.org> Date: Sat Sep 08 00:23:44 2018 WebUI: Fix cr-input to work with Polymer 2. In Polymer 2 the following syntax for registering event listeners is no longer supported. listeners: { 'id.eventName': 'onEventName_', }, Instead the listener must be added either programmatically, or declaratively directly on the |id| element. Bug:875470 Change-Id: I84f1d61b9b669dda1922f6c8d5f2bbc8de211c1c Reviewed-on: https://chromium-review.googlesource.com/1213380 Reviewed-by: Esmael El-Moslimany <aee@chromium.org> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#589732} [modify] https://crrev.com/142d451318f046a97c9df1c2459226f1f0e95afd/testing/buildbot/filters/webui_polymer2_interactive_ui_tests.filter [modify] https://crrev.com/142d451318f046a97c9df1c2459226f1f0e95afd/ui/webui/resources/cr_elements/cr_input/cr_input.html [modify] https://crrev.com/142d451318f046a97c9df1c2459226f1f0e95afd/ui/webui/resources/cr_elements/cr_input/cr_input.js
,
Sep 8
@rbpotter: The remaning test mentioned in the opening comment seem to be enabled already, see [1]. Were they originally disabled, or were they always flaky but enabled? Trying to determine if I can mark this bug as Fixed. [1] https://cs.chromium.org/chromium/src/testing/buildbot/filters/webui_polymer2_interactive_ui_tests.filter?l=7,9-10
,
Sep 10
Yes, it appears those flaky tests were actually only flaky on a Windows bot that does not run interactive_ui_tests either, so I re-enabled them after pulling all the Polymer 2 interactive_ui_tests off of that bot. See https://crrev.com/c/1187729. I think this can now be marked as fixed since the 2 other failing tests are fixed. |
||||
►
Sign in to add a comment |
||||
Comment 1 by rbpotter@chromium.org
, Aug 17