New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 735633 link

Starred by 6 users

Issue metadata

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

Blocking:
issue 523952



Sign in to add a comment

WebUI handle style in HTML imports deprecation warning.

Project Member Reported by dpa...@chromium.org, Jun 21 2017

Issue description

See details at https://github.com/TakayoshiKochi/deprecate-style-in-html-imports. There is a new warning in the dev console of most WebUI pages

[Deprecation] Styling master document from stylesheets defined in HTML Imports is deprecated, and is planned to be removed in M65, around March 2018. Please refer to https://goo.gl/EGXzpw for possible migration paths.

The deprecation is tracked by https://bugs.chromium.org/p/chromium/issues/detail?id=523952.
 

Comment 1 by dpa...@chromium.org, Jun 21 2017

Blocking: 523952

Comment 2 by dpa...@chromium.org, Jun 22 2017

Cc: rdevlin....@chromium.org
@kochi: It  would be nice if the warning itself included the name of the files that cause it.

Either way, I looked inside Polymer a bit and found some potential violations.

 1) https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/components-chromium/iron-flex-layout/classes/iron-flex-layout.html?l=11 (we've already removed usage of this from most MD pages as part of  issue 635633 .
 2) https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/components-chromium/paper-styles/default-theme.html?l=16

There could be more, will keep looking.

Comment 3 by dpa...@chromium.org, Jun 22 2017

A few more (I think):
paper-styles/typography.html
paper-styles/classes/typography.html
paper-styles/classes/global.html
paper-styles/shadow.html
paper-styles/color.html

@kochi: Can you verify if all those are violations? Also given that these are in Polymer itself (and most likely used by Polymer elements themselves), this is going to need some collaboration from the Polymer team to remove.

Comment 4 by kochi@chromium.org, Jun 23 2017

@dpapad: thanks for the suggestion, I'll work on showing which import file is trying
to add stylesheet.

And yes, they look like just using the polymer elements as is or slight modification,
I'll also talk to Polymer people.

The way to go would be to upstream what is found here to original polymer
elements, as well as to import what is updated in upstream.

FYI, here's one issue filed against polymer element (custom-style)
https://github.com/Polymer/polymer/issues/4679

Comment 5 by dpa...@chromium.org, Jun 30 2017

FYI, I used https://chromium-review.googlesource.com/c/557781/ by kochi to get a list of offending files. Pasting below (built with use_vulcanize=false)

# cr-elements
chrome://resources/cr_elements/policy/cr_policy_pref_indicator.html
chrome://resources/cr_elements/policy/cr_policy_vars_css.html
chrome://resources/cr_elements/shared_vars_css.html

# settings
chrome://md-settings/controls/settings_radio_group.html
chrome://md-settings/passwords_and_forms_page/credit_card_edit_dialog.html
chrome://md-settings/settings_vars_css.html

# history
chrome://history/app.html
chrome://history/shared_vars.html

# extensions
chrome://extensions/detail_view.html
chrome://extensions/icons.html
chrome://extensions/item.html
chrome://extensions/manager.html
chrome://extensions/toolbar.html

# pdf viewer
chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/elements/shared-vars.html

# Polymer
chrome://resources/polymer/v1_0/font-roboto/roboto.html
chrome://resources/polymer/v1_0/iron-flex-layout/iron-flex-layout.html
chrome://resources/polymer/v1_0/paper-styles/color.html
chrome://resources/polymer/v1_0/paper-styles/default-theme.html
chrome://resources/polymer/v1_0/paper-styles/shadow.html
chrome://resources/polymer/v1_0/paper-styles/typography.html
Cc: calamity@chromium.org
Cc: dpa...@chromium.org
 Issue 793529  has been merged into this issue.

Comment 8 by kochi@chromium.org, Dec 22 2017

FYI, the latest Polymer <custom-style> elements (and maybe others?)
have a workaround for moving any <style> element into main document,
but still produces the warning message at the first parsing of the
HTML Import content when it still has the <style> element.

Comment 9 by kochi@chromium.org, Dec 26 2017

@dpapad now M65 is in development, in which we plan to remove <style>
application from HTML Imports... could you sanity check if WebUI code
work with --disable-blink-features=HTMLImportsStyleApplication?

With the flag disabled, all the tests pass now.
https://chromium-review.googlesource.com/c/chromium/src/+/842283
(but does not guarantee all the style is not broken?)

I casually tested bookmark manager, download shelf, settings but nothing
was broken.
@kochi: Did a quick test and found that the new MD Extensions (--enable-features=MaterialDesignExtensions) is broken with --disable-blink-features=HTMLImportsStyleApplication, see attached screenshot.

Have not investigated further yet.
md_extensions_broken.png
68.3 KB View Download
Note that I can only repro the breakage with the following GN flag

optimize_webui = false
@dpapad what does optimize_webui mean, and is "true" default?

Let me know what are broken with the default settings (with
--disable-blink-features=HTMLImportsStyleApplication), and
what are broken which are not enabled by default?

Anyway, the intent to remove has some questions in blink-dev,
I don't think we can remove the feature in M65.

Let's figure this out.
> @dpapad what does optimize_webui mean, and is "true" default?

optimize_webui flag controls whether MD WebUIs are optimized using polymer-bundler, polymer-css-build and uglify. Essentially those tools bundle everything into one HTML and one JS file, such that there are no HTML imports in "release" builds.

The default value if none provided is "true". All CQ tryjobs are using "true" except for debug builders/testers which use "false".

I'll investigate further tomorrow about why it is broken, and what else is broken. FWIW, Settings page did not seem broken with either of optimize_webui true/false.
Findings:
There are several HTML imports, see [1] that contain the stylesheet at [2]. Those stop working with the flag, breaking the style. 

History and Bookmarks besides including this stylesheet from HTML imports, they also include it from the top-level file, which is not an HTML import, and therefore do not look broken, see [3] and [4].

[1] https://cs.chromium.org/search/?q=%3Clink+rel%3D%22stylesheet%22+href%3D%22chrome://resources/css/md_colors.css%22%3E&sq=package:chromium&type=cs
[2] https://cs.chromium.org/chromium/src/ui/webui/resources/css/md_colors.css
[3] https://cs.chromium.org/chromium/src/chrome/browser/resources/md_bookmarks/bookmarks.html?l=7
[4] https://cs.chromium.org/chromium/src/chrome/browser/resources/md_history/history.html?l=8
FYI candidate fix for the cases mentioned above is at https://chromium-review.googlesource.com/c/chromium/src/+/853138.

An alternative fix (I think), would be to move the contents of md_colors.css within [1]

[1] https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/shared_vars_css.html.
Project Member

Comment 16 by bugdroid1@chromium.org, Jan 8 2018

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

commit 5f4ce8d92827e65b4049af367de043f1f6b02c3d
Author: dpapad <dpapad@chromium.org>
Date: Mon Jan 08 19:24:13 2018

WebUI: Remove <link rel="stylesheet"> from HTML imports.

This fixes the styling when --disable-blink-features=HTMLImportsStyleApplication
is used (which will become the default soon).

Bug: 735633
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ia739eca23534e0f5a4e9ff5eeb3d124e45d98687
Reviewed-on: https://chromium-review.googlesource.com/853138
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527708}
[modify] https://crrev.com/5f4ce8d92827e65b4049af367de043f1f6b02c3d/chrome/browser/resources/md_bookmarks/toolbar.html
[modify] https://crrev.com/5f4ce8d92827e65b4049af367de043f1f6b02c3d/chrome/browser/resources/md_extensions/extensions.html
[modify] https://crrev.com/5f4ce8d92827e65b4049af367de043f1f6b02c3d/chrome/browser/resources/md_extensions/manager.html
[modify] https://crrev.com/5f4ce8d92827e65b4049af367de043f1f6b02c3d/chrome/browser/resources/md_extensions/toolbar.html
[modify] https://crrev.com/5f4ce8d92827e65b4049af367de043f1f6b02c3d/chrome/browser/resources/md_history/app.html

Comment 17 by kochi@chromium.org, Jun 19 2018

dpapad@ or anyone: Is there remaining task for this in WebUI?

We are abandoning the idea of removing styling feature from HTML Imports,
and are targeting to deprecate entire HTML Imports.
(See the reasoning https://bugs.chromium.org/p/chromium/issues/detail?id=523952#c55)
I still see the warning for this when loading Chrome's PDF viewer (and, by extension, Print Preview).
WebUI still relies heavily on HTML imports (both in debug and release mode). We should have a separate discussion about this roadmap (expecting this could take a while, as we are still working on migrating to Shadow DOM v1).

Regarding remaining warnings, I still see them in Settings, Downloads, History. I would have to re-investigate to see what is left to do there. Is it important to remove these warnings, given the new direction?

Comment 20 by kochi@chromium.org, Jun 20 2018

Cc: yoichio@chromium.org
+yoichio@
I don't want to stop deprecation message itself because it looks Chrome
stopped to deprecation somehow for web authors.

I'm happy to help you removing HTML import work.
Do you want more good deprecation message like
"[Deprecation] xxx.html has styles but Styling master document from stylesheets defined in HTML Imports is deprecated,,,".
But I feel it is bit noisy,,,WDYK?

FWIW, the deprecation message is only shown when running with optimize_webu=false for most WebUIs. optimize_webui=true eliminates the message.

I don't know if there is anything to do from the WebUI side about this deprecation message. It seems that it's not important, and we should be focusing on long term HTML import removal instead. In that sense, I don't have any opinions about the wording of that message.

Sign in to add a comment