New issue
Advanced search Search tips

Issue 724597 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 860069



Sign in to add a comment

Test.disableAnimationsAndTransitions uses /deep/ in CSS rules.

Project Member Reported by dpa...@chromium.org, May 19 2017

Issue description

See https://cs.chromium.org/chromium/src/chrome/test/data/webui/test_api.js?l=55,59.

/deep/ in CSS selectors is about to be removed at  issue 489954 . We should figure out if disableAnimationsAndTransitions() needs to be completely removed or updated in some equivalent way but without using /deep/.
 
I noticed that the warning now says that /deep/ is a no-op

"[Deprecation] /deep/ combinator is no longer supported in CSS dynamic profile. It is now effectively no-op, acting as if it were a descendant combinator. You should consider to remove it. See https://www.chromestatus.com/features/4964279606312960 for more details."

@dbeam: I am wondering if this is contributing to any newly found test flakiness.

FWIW, here is a list of places where disableAnimationsAndTransitions() is currently used at 

chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
chrome/test/data/webui/print_preview/print_preview_ui_browsertest.js
chrome/test/data/webui/settings/android_apps_page_test.js
chrome/test/data/webui/settings/bluetooth_page_tests.js
chrome/test/data/webui/settings/device_page_tests.js
chrome/test/data/webui/settings/internet_page_tests.js
chrome/test/data/webui/settings/languages_page_tests.js

We should probably come up with a plan on how to disable animations without using /deep/.

Cc: rbpotter@chromium.org
+rbpotter as FYI, since one of those tests is print_preview_ui_browsertest.js
Owner: dbeam@chromium.org
Status: Started (was: Available)
Fix candidate at https://codereview.chromium.org/2927663003/.
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2deb8c3767fbb87c7fac334eda0624bc838903df

commit 2deb8c3767fbb87c7fac334eda0624bc838903df
Author: dbeam <dbeam@chromium.org>
Date: Thu Jun 08 00:38:36 2017

Fix Test.disableAnimationsAndTransitions to work with new /deep/

R=hcarmona@chromium.org
BUG=724597

Review-Url: https://codereview.chromium.org/2927663003
Cr-Commit-Position: refs/heads/master@{#477834}

[modify] https://crrev.com/2deb8c3767fbb87c7fac334eda0624bc838903df/chrome/test/data/webui/test_api.js

It's worth noting that the previous code would apply for all elements that are created during the lifetime of the test. Whereas the current solution will only affect elements that are already in the DOM. I don't know if we plan to address this (or defer until it proves to be an issue).

Comment 6 by dbeam@chromium.org, Jun 8 2017

right, potential ways to do do this would be

1) listen for dom-change events (for polymer code, assuming they're on)
2) use a MutationObserver
MutationObserver observes a single HTMLElement and only reports DOM changes of its immediate children AFAIK.

Comment 9 by dbeam@chromium.org, Jun 8 2017

yeah, sorry, you're right: I thought childList was recursive.

Comment 10 by dbeam@chromium.org, Jun 30 2017

Owner: ----
Status: Available (was: Started)
Blocking: -489954
Labels: -Pri-2 Pri-3
This is a no longer blocking issue. /deep/ and ::shadow were removed at M63.
Blocking: 860069

Sign in to add a comment