Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update custom-style to hoist styles to main document #4679

Closed
sorvell opened this issue Jun 9, 2017 · 21 comments
Closed

Update custom-style to hoist styles to main document #4679

sorvell opened this issue Jun 9, 2017 · 21 comments

Comments

@sorvell
Copy link
Contributor

sorvell commented Jun 9, 2017

Because stylesheets in HTMLImports will be deprecated in Chrome, the custom-style element should be updated to hoist style elements into the main document where they will apply as expected. See https://bugs.chromium.org/p/chromium/issues/detail?id=523952.

@sorvell sorvell self-assigned this Jun 9, 2017
@sorvell sorvell changed the title Update custom-style to hoist styles to make document Update custom-style to hoist styles to main document Jun 9, 2017
@sorvell sorvell changed the title Update custom-style to hoist styles to main document Update custom-style to hoist styles to main document Jun 9, 2017
@dfreedm dfreedm self-assigned this Jun 17, 2017
@jifalops
Copy link

Looking forward to an example of system wide styles for theming. Will it still be possible to include styles via <custom-style include="...">?

@pdesjardins90
Copy link

[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.

=>

For Polymer (both 1.x and 2.x) users, custom-style usage is affected by this. The framework will be updated to handle this accordingly. Follow #4679 for the details.

So if I understand correctly, this warning will be solved automatically when you release a new version of <custom-style>?

@arthurevans
Copy link

Ping @azakus @sorvell this is coming up more now that the deprecation message is showing up in Stable. Do we have a timeframe for addressing this?

@dfreedm
Copy link
Member

dfreedm commented Sep 20, 2017

Thanks for the ping! I'll try to get this out the door by next week.

dfreedm added a commit that referenced this issue Sep 20, 2017
Work around deprecation of styles in HTML Imports
https://crbug.com/523952

Fixes #4679
dfreedm added a commit that referenced this issue Sep 20, 2017
Work around deprecation of styles in HTML Imports
https://crbug.com/523952

Backport from 155ab8a for 2.x

Related to #4679
@ggriffithsIDBS
Copy link

ggriffithsIDBS commented Sep 29, 2017

So 1.10.1 just got released but im still seeing this warning all over. Would be so much better if had some idea what files were causing this. Seeing it on my demo pages, with the iron-demo-helpers components. Need to see if its our code or not too.

Im seeing this warning when i use iron-icon in elements too

Its also hard to tell if this deprecation is a breaking change, or if the behaviour will fall back to the polyfill?
Not everyone has applications that can just have new versions deployed to customers immediately, and since Chrome is a moving version target too, We will be left with not many options other than patching when this is removed from the browser (if it breaks things)

@moderndeveloperllc
Copy link

@sorvell 2.1.1 also still shows the warning. It might be nice to know what this fix does and does not do in regards to "fixing" the depreciation warning.

@dfreedm
Copy link
Member

dfreedm commented Sep 29, 2017

@ggriffithsIDBS @moderndeveloperllc The deprecation notice mentions that <style> in an HTML Import will no longer apply to the main document in Chrome 65+, which should first roll out around March 2018. This patch makes sure that in Polymer 1.10.1 and 2.1.1 that any <style is="custom-style"> or <custom-style></custom-style> will be moved to the main document when they boot to continue having the behavior currently available.

Now, as for the logging of the warning, I don't think there's much I can do. The styles should move when they boot, but this is likely after the browser check for styles in HTML Import documents occurs, meaning that there's not much I can do about it.

Using polymer build or vulcanize should remove the warning because of the act of bundling, but the styling behavior people expect to occur with Polymer should continue to work even without this step.

@arthurevans
Copy link

I don't think it will suppress the warning, but it should prevent problems when the change rolls out.

We're planning on publishing a blog post with guidance about this and the coming change to /deep/. I'll check in with @azakus and @sorvell and try to get that out today or early next week.

@arthurevans
Copy link

Or @azakus could just answer the question right here 😀.

@arthurevans
Copy link

@azakus I think that vulcanizing or using polymer-build will only remove the warning if the imports in question are bundled into the main document, which is not always the case.

(In the case of polymer build, for example, they'd typically be bundled into either the app shell bundle or a fragment bundle, and still imported as part of an HTML import.)

@ensonic
Copy link

ensonic commented Oct 10, 2017

I switched to "Polymer/polymer#^1.10.1" and still get the "[Deprecation] Styling master document from stylesheets defined in HTML Imports is deprecated ..." warning. If this is caused by a webcomponent, how do I find out which one it is? I've updated them all to the latest version on their 1.X branches, moving to polymer 2.X is next but needs more work.

@dfreedm
Copy link
Member

dfreedm commented Oct 10, 2017

Yes, you're going to get that for now because the deprecation notice happens right after parsing of the page, before the custom style can upgrade and move into the main document.

We're working on some things we could do with the polymer CLI to remove the deprecation notice, but that work is still ongoing.

openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Nov 14, 2017
Reason why is there is an upcoming breaking change in chrome which
will break things to do with styles.

I've created this fix I967aa12a7ead22650940caf66bb4fa61256992ac

But [1] recommends us be on polymer 1.10.1+. It includes this fix [2].

[1] https://github.com/TakayoshiKochi/deprecate-style-in-html-imports

[2] Polymer/polymer#4679

Change-Id: I8f8490e6644f9671a4d81751ab21d8ef5fe146fd
@kevinpschaaf
Copy link
Member

I've opened #5017 to allow users to avoid the deprecation warning by adding a type="custom" attribute to custom style <style> tags to prevent them from being parsed, which avoids the deprecation warning.

@rnarasap27
Copy link

rnarasap27 commented Jan 17, 2018

Does this also impact the below ?

<style include="shares-styles"></style>

And this is inside the template element and in Polymer 1.0

@billhimmelsbach
Copy link

Should I now be able to see the effects of this deprecation if I test my site in Canary? (65.0.3323.0)

@kobleistvan
Copy link

@billhimmelsbach Seems that Canary 66 cahnged the message to May:
[Deprecation] Styling master document from stylesheets defined in HTML Imports is deprecated, and is planned to be removed in M67, around May 2018. Please refer to https://goo.gl/EGXzpw for possible migration paths.

@yoichio
Copy link

yoichio commented Mar 22, 2018

Hi, I'm an owner of HTML Import in Blink.
I used to be an owner but has been away from the work and came back again.

I originally introduced the flag WebFeature::kHTMLImportsHasStyleSheets to just
count style inside imported documents on load and not for counting real usage.
I think warning deprecation on the flag is little overkill and annoying.

What kind of message is better and how/when should we show it?

@TakayoshiKochi
Copy link

How about showing the message when the stylesheet is actually applied to the document?

The message was confusing because it was shown even when an workaround is applied.

@kobleistvan
Copy link

Agree with @TakayoshiKochi , that would actually be useful info.

@brettpostin
Copy link

The warnings are putting us in a state of limbo right now. Faced with the deprecation, we want to fix up our apps and element suites asap. However the false positives are making us hold off as we aren't confident in our coverage. Additionally some of the PolymerElements we use trigger these warnings (e.g. app-drawer > iron-flex-layout) and it is time consuming getting to the bottom of the warning source.

It would be really helpful if one of two things could happen (and fairly quickly with the deprecation deadline looming):

  1. @TakayoshiKochi suggestion of triggering the warning when the stylesheet is applied. Maybe with additional info of the source import so we can decide whether to ignore?
  2. Merge @kevinpschaaf [2.x] Allow adding type="custom" to avoid deprecation warning. Fixes #5017 #5018 which will help with our own elements but would also need to see the fix applied to the relevant PolymerElements.

@lfaz
Copy link

lfaz commented Oct 29, 2018

This is not working for me still in Chrome:

var printWindow = window.open('', 'PrintWindow', "width="+screen.availWidth+", ```
height="+screen.availHeight);
    printWindow.document.write('<html><head>');
    printWindow.document.write('<link rel="stylesheet" href="/css/styles.css">');
    printWindow.document.write('</head>');
    printWindow.document.write('<body>');
    printWindow.document.write(document.getElementById(resultId).innerHTML);
    printWindow.document.write('</body></html>');
    printWindow.document.close(); // necessary for IE >= 10
    printWindow.focus(); // necessary for IE >= 10*/
    printWindow.print();
    printWindow.close();
    return true;

Any idea how to fix? It works on Firefox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests