<link rel="stylesheet" disabled> #3840
Comments
|
I guess this changed in cc5fa75... |
|
I think it is fine to leave this unchanged spec-wise at least until there's more compat data. https://bugs.chromium.org/p/chromium/issues/detail?id=695984 was already on file to remove the attribute from Blink. I can try to remove the WebIDL from Firefox on Nightly or something and see if there's fallout as well. |
|
This area was last investigated in #1081, FWIW. |
|
cc @foolip, do you think it'd be reasonable to remove the The use counters look somewhat high: https://www.chromestatus.com/metrics/feature/timeline/popularity/809 But Blink's behavior is clearly bogus and not something we'd want to implement, in the sense that it's not on Alternatively, we should decide whether disabled links appear on document.styleSheets, whether they trigger loads (they don't seem to in Blink), etc. I'm happy to do that too but it seems fairly more work. |
|
Also it's really weird that |
|
I would slightly hope that we don't add it, but if Blink is unwilling to remove it and it'd be required by webcompat, I guess it's fine to add... We should probably sync this carefully with the existing disabled state in CSSOM. |
|
I think if we spec it it may be worth to prevent it from triggering loads and appearing in That'd be consistent with current Blink / WebKit's behavior when there's no dynamic fiddling: Though I'd too prefer not having to do it as well, it'd be somewhat confusing either way. |
|
I've noticed this from time to time but never known quite what to make of it, or what the compat constraints are. In https://bugs.chromium.org/p/chromium/issues/detail?id=695984&desc=2#c13 I wrote:
The oddity here is that reflecting the content attribute doesn't mix with forwarding to the element's sheet, and Blink is inconsistent in what it does. If the attributes are reflected, then it will be possible for them to be out of sync with the real disabled state of the sheets, unless setting There are also more related use counters:
All of them have fairly high usage, but that doesn't say too much about the web compat constraints. Concrete options that come to mind:
In both cases, what to do if |
Yes. Afaik sites use it, and @foolip's claim about use counters agrees. I don't understand why it was ever removed from the spec. :( I did some quick testing, and setting My personal, no-hysteresis, preference is that the content attribute does nothing and the DOM attribute forwards to the underlying sheet, if any... I admit that I'm biased, since that's what Gecko implements. One comment about the above use counter data. https://www.chromestatus.com/metrics/feature/timeline/popularity/809 includes all the sites setting |
|
I guess #1081 (comment) talks a bit about extracting signal from a combination of that use counter and the V8HTMLLinkElement_Disabled_AttributeSetter use counter... |
No, you're right about that, when https://www.chromestatus.com/metrics/feature/timeline/popularity/811 is triggered, https://www.chromestatus.com/metrics/feature/timeline/popularity/809 will be too. They're in a very similar range, so most likely the content attribute counter is dominated by the IDL attribute being said. (It'd be consistent with the data that pages are doing both, but less plausible.) I'll update the use counter to measure the parser case separately. |
|
Sent https://chromium-review.googlesource.com/c/chromium/src/+/1148203, but honestly I'm not too optimistic about this alone telling us whether the content attribute is needed for real-world compat. I did a quick httparchive search for the regexp page,url http://www.hacontent.com/,http://www.aon.com/human-capital-consulting/default.jsp http://www.waxfilm.net/,http://xxxx.re/ http://www.manufaktura.com/,http://www.manufaktura.com/ http://www.trl.org/,http://www.trl.org/ http://www.isiment.com/,http://www.isiment.com/ http://www.aon.com/,http://www.aon.com/default.jsp http://www.skiphop.com/,http://www.skiphop.com/ http://www.skiphop.com/,http://www.skiphop.com/?callback=located&_=1517679261400 http://www.skiphop.com/,http://www.skiphop.com/home?id=skiphop&_=1517679261400&callback=located http://www.ksepb.gov.tw/,http://www.ksepb.gov.tw/ http://www.ordistricts.nic.in/,http://www.ordistricts.nic.in/ http://www.maarefquran.org/,http://www.maarefquran.org/ http://www.mosconsv.ru/,http://www.mosconsv.ru/ http://www.otsuka-plus1.com/,http://www.otsuka-plus1.com/ http://www.apkpro.ru/,http://www.apkpro.ru/ http://www.dxbsky.com/,http://www.dxbsky.com/ http://www.supadipa.com/,http://www.supadipa.com/ http://www.learncodeonline.in/,http://www.learncodeonline.in/ http://www.clien.net/,http://www.clien.net/ http://www.funai.gov.br/,http://www.funai.gov.br/ http://www.oshkosh.com/,http://www.oshkosh.com/ http://www.nomade.kr/,http://www.nomade.kr/ http://www.rev.cn/,http://www.rev.cn/ http://www.rev.cn/,http://www.rev.cn/js/header.js http://www.shasm.gov.cn/,http://www.shasm.gov.cn/JScript.js http://www.xn--80aaaac8algcbgbck3fl0q.xn--p1ai/,http://www.xn--80aaaac8algcbgbck3fl0q.xn--p1ai/ http://www.scorecardrewards.com/,https://www.scorecardrewards.com/assets/hinda-0.0.1.11756.js http://www.gate.io/,http://www.gate.io/ http://www.gate.io/,https://gate.io/ http://www.cpcb.nic.in/,http://www.cpcb.nic.in/ http://www.aon.fr/,http://www.aon.fr/france/default.jsp http://www.dajs.gov.cn/,http://www.dajs.gov.cn/ http://www.chilicel.com/,http://www.chilicel.com/ http://www.steamspy.com/,http://www.steamspy.com/ http://www.hainanairlines.com/,http://www.hainanairlines.com/distil_identify_cookie.html?httpReferrer=%2F&uid=8FD9C2E9-ED92-3AD1-97CC-88143764D73C http://www.myrunningman.com/,http://www.myrunningman.com/ http://www.sanwa-p.co.jp/,https://www.sanwa-p.co.jp/js/jquery.font.js http://www.teenarea.biz/,http://www.teenarea.biz/ http://www.wlnupdates.com/,http://www.wlnupdates.com/ http://www.yourtotalrewards.com/,http://www.aon.com/human-capital-consulting/default.jsp http://www.cbenni.com/,http://www.cbenni.com/ http://www.mphro.com/,http://www.aon.com/human-capital-consulting/default.jsp http://www.kkesh.med.sa/,http://www.kkesh.med.sa/ http://www.connected2.me/,http://www.connected2.me/ http://www.microfocus.net/,http://www.microfocus.net/ http://www.ficpa.org/,http://www.ficpa.org/ http://www.supeera.com/,http://www.supeera.com/ http://www.pesc.ru/,http://www.pesc.ru/ http://www.synchroarts.com/,http://www.synchroarts.com/ http://www.dxbpp.gov.ae/,http://www.dxbpp.gov.ae/ http://www.vodafonefreezone.com/,http://www.vodafonefreezone.com/ http://www.tmbam.com/,https://www.tmbam.com/home/th/inc/setFont.js http://www.jnu.ac.in/,http://www.jnu.ac.in/ http://www.bemlindia.com/,http://www.bemlindia.com/ http://www.fcijobportalmah.com/,http://fcijobportalmah.com/FCI01/js/FontSizes.js http://www.aon.it/,http://www.aon.com/italy http://www.mkb.hu/,http://www.mkb.hu/ http://www.carters.com/,http://www.carters.com/ http://www.szhr.com.cn/,http://www.szhr.com.cn/hrmall/index-center.html http://www.szhr.com.cn/,http://www.szhr.com.cn/hrmall/index_ad_item_hr.html http://www.alsok.co.jp/,http://www.alsok.co.jp/ http://www.wum.edu.pl/,http://www.wum.edu.pl/themes/frameworkNOWY/kontrast.js?V http://www.wum.edu.pl/,http://www.wum.edu.pl/modules/styleswitcher/styleswitcher.js http://www.cartersoshkosh.ca/,http://www.cartersoshkosh.ca/ http://www.cartersoshkosh.ca/,http://www.cartersoshkosh.ca/?callback=located&_=1517713421387 http://www.cartersoshkosh.ca/,http://www.cartersoshkosh.ca/on/demandware.store/Sites-CartersCA-Site/en_CA/Default-Start?callback=located&_=1517713421387 http://www.hewitt.com/,http://www.aon.com/human-capital-consulting/default.jsp http://www.jeld-wen.com/,http://www.jeld-wen.com/ http://www.dotajackpots.com/,http://www.dotajackpots.com/ http://www.cvag.de/,http://www.cvag.de/ http://www.triptogether.com/,http://www.triptogether.com/ http://www.oddistricts.nic.in/,http://www.oddistricts.nic.in/ http://www.previdencia.gov.br/,http://www.previdencia.gov.br/ http://www.bjft.gov.cn/,http://www.bjft.gov.cn/top_lan.html That's not a lot, but there is some stuff in there that looks like deliberate use. On https://www.ksepb.gov.tw/ the disabled attribute is everywhere, and then there's some scripts that poke at the IDL attributes too. Unfortunately, this page doesn't load in Firefox at all due to an invalid cert, so I can't check if it's broken or not. Spending a couple of hours on this list and comparing the result in a Chromium build with or without the disabled content attribute supported should give a good idea about the risks. Before I go spend a bunch of time on that I'd like to hear if the Edge or WebKit team have thoughts on this. @patrickkettner @cdumez, can you find contacts? |
|
Looking a bit more, there's a layer between the disabled content attribute and underlying sheet which would explain the differences in This dates back to 2002/2003 when Dave Hyatt exclaimed that "This disabling stuff is complicated!" Because of the reflection, this stuff is in play when setting the |
The existing kHTMLLinkElementDisabled counter is poisoned by the reflected IDL attribute, as pointed out by bz: whatwg/html#3840 (comment) Change-Id: Ic4917290475f1ad6d9baba06620a485b78edac59 Reviewed-on: https://chromium-review.googlesource.com/1148203 Reviewed-by: Mike West <mkwst@chromium.org> Commit-Queue: Philip Jägenstedt <foolip@chromium.org> Cr-Commit-Position: refs/heads/master@{#577870}
|
Here's a concrete proposal: Make This is a nice way of preventing stylesheet loads, and is the sanest thing I could think of, since @foolip does that sound reasonable? It means that using the getter and setter produces the same result rendering-wise, and consistent results regardless of whether the attribute was set from the parser or not. |
|
Ugh, the only reason that people use |
|
So, ok, a bit more of an elaborated answer (I'll use WebKit and Blink interchangeably here). We (Gecko) get compat issues from not supporting This attribute is mostly used in along with For example, there's no other way to get the stylesheet as follows to apply in WebKit other than changing the <link rel="alternate stylesheet" href="data:text/css,html { background: green }">
So for convenience, and in order to be able to toggle alternate stylesheets across browsers, people do the following: <link rel="alternate stylesheet" href="data:text/css,html { background: green }" disabled>So that they can enable the stylesheet using There are a whole lot of extra issues in WebKit implementation, like the stylesheet inconsistently appearing in Proposed modelThe model I proposed above tries to get the benefits of the current
So with that, the proposal is:
Does that sound reasonable to everybody? |
|
"reasonable" is a high bar, but if WebKit/Blink are not going to fix their bugs around alternate stylesheets this sounds like the next-best option to me... |
|
https://bugzilla.mozilla.org/show_bug.cgi?id=1281135 has a patch implementing that on Gecko, fwiw. |
|
I agree we should do something to improve interoperability here. @emilio : When you say "WebKit" in #3840 (comment) you mean both WebKit and Blink? If so, changing the behavior of WebKit and Blink to match the spec might pose a compatibility risk. In particular, removing & adding an item from |
Yes.
Right now the WebKit / Blink behavior doesn't really make much sense. If you use I think it's also a bug that in WebKit / Blink you cannot enable an alternate stylesheet without the I think what I propose is a good way to get interop with a somewhat consistent model. But I'm all ears if you have better ideas. Given the poor interop situation here I think we have some leeway... |
…reflect that attribute. r=bzbarsky ...instead of forwarding to the sheet like HTMLStyleElement does. I've proposed this behavior in: whatwg/html#3840 (comment) I think this is one of the sane behaviors we can have, what Blink / WebKit do makes no sense to me. Alternative potentially-sane behavior is making the initial value of the stylesheet's disabled bit the same as the content attribute, and both reflect and forward the attribute from the setter. That means that setAttribute does something different than setting `disabled`, which means that you can get into all sorts of funny states when reloading the sheet... So I rather not do that. Differential Revision: https://phabricator.services.mozilla.com/D26573 --HG-- extra : moz-landing-system : lando
…reflect that attribute. r=bzbarsky ...instead of forwarding to the sheet like HTMLStyleElement does. I've proposed this behavior in: whatwg/html#3840 (comment) I think this is one of the sane behaviors we can have, what Blink / WebKit do makes no sense to me. Alternative potentially-sane behavior is making the initial value of the stylesheet's disabled bit the same as the content attribute, and both reflect and forward the attribute from the setter. That means that setAttribute does something different than setting `disabled`, which means that you can get into all sorts of funny states when reloading the sheet... So I rather not do that. Differential Revision: https://phabricator.services.mozilla.com/D26573
At some point, the link element's disabled attribute was removed from the spec. It has since been restored but was not linked to the stylesheet's disabled attribute. In whatwg/html#3840, the two are being revised to reflect one another. This BCD update adds data to indicate this.
* Update for link element's disabled attribute being added back At some point, the link element's disabled attribute was removed from the spec. It has since been restored but was not linked to the stylesheet's disabled attribute. In whatwg/html#3840, the two are being revised to reflect one another. This BCD update adds data to indicate this. * First bit of corrections for emilio * Further fixes * Apply Chrome information to other browsers, with minor rewording to be inclusive of all Blink-based products. Replaced old and incomplete note on the Android WebView entry.
…ttribute. ...instead of forwarding to the sheet like HTMLStyleElement does. I've proposed this behavior in: whatwg/html#3840 (comment) I think this is one of the sane behaviors we can have, what Blink / WebKit do makes no sense to me. Alternative potentially-sane behavior is making the initial value of the stylesheet's disabled bit the same as the content attribute, and both reflect and forward the attribute from the setter. That means that setAttribute does something different than setting `disabled`, which means that you can get into all sorts of funny states when reloading the sheet... So I rather not do that. Differential Revision: https://phabricator.services.mozilla.com/D26573 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1281135 gecko-commit: edbf9758ea946df5063e84c397c43208c555fd74 gecko-integration-branch: central gecko-reviewers: bzbarsky
…reflect that attribute. r=bzbarsky ...instead of forwarding to the sheet like HTMLStyleElement does. I've proposed this behavior in: whatwg/html#3840 (comment) I think this is one of the sane behaviors we can have, what Blink / WebKit do makes no sense to me. Alternative potentially-sane behavior is making the initial value of the stylesheet's disabled bit the same as the content attribute, and both reflect and forward the attribute from the setter. That means that setAttribute does something different than setting `disabled`, which means that you can get into all sorts of funny states when reloading the sheet... So I rather not do that. Differential Revision: https://phabricator.services.mozilla.com/D26573 UltraBlame original commit: ef991fe893a435f4416bde5f518135655a9740e1
…reflect that attribute. r=bzbarsky ...instead of forwarding to the sheet like HTMLStyleElement does. I've proposed this behavior in: whatwg/html#3840 (comment) I think this is one of the sane behaviors we can have, what Blink / WebKit do makes no sense to me. Alternative potentially-sane behavior is making the initial value of the stylesheet's disabled bit the same as the content attribute, and both reflect and forward the attribute from th 6D40 e setter. That means that setAttribute does something different than setting `disabled`, which means that you can get into all sorts of funny states when reloading the sheet... So I rather not do that. Differential Revision: https://phabricator.services.mozilla.com/D26573 UltraBlame original commit: edbf9758ea946df5063e84c397c43208c555fd74
…reflect that attribute. r=bzbarsky ...instead of forwarding to the sheet like HTMLStyleElement does. I've proposed this behavior in: whatwg/html#3840 (comment) I think this is one of the sane behaviors we can have, what Blink / WebKit do makes no sense to me. Alternative potentially-sane behavior is making the initial value of the stylesheet's disabled bit the same as the content attribute, and both reflect and forward the attribute from the setter. That means that setAttribute does something different than setting `disabled`, which means that you can get into all sorts of funny states when reloading the sheet... So I rather not do that. Differential Revision: https://phabricator.services.mozilla.com/D26573 UltraBlame original commit: ef991fe893a435f4416bde5f518135655a9740e1
…reflect that attribute. r=bzbarsky ...instead of forwarding to the sheet like HTMLStyleElement does. I've proposed this behavior in: whatwg/html#3840 (comment) I think this is one of the sane behaviors we can have, what Blink / WebKit do makes no sense to me. Alternative potentially-sane behavior is making the initial value of the stylesheet's disabled bit the same as the content attribute, and both reflect and forward the attribute from the setter. That means that setAttribute does something different than setting `disabled`, which means that you can get into all sorts of funny states when reloading the sheet... So I rather not do that. Differential Revision: https://phabricator.services.mozilla.com/D26573 UltraBlame original commit: edbf9758ea946df5063e84c397c43208c555fd74
…reflect that attribute. r=bzbarsky ...instead of forwarding to the sheet like HTMLStyleElement does. I've proposed this behavior in: whatwg/html#3840 (comment) I think this is one of the sane behaviors we can have, what Blink / WebKit do makes no sense to me. Alternative potentially-sane behavior is making the initial value of the stylesheet's disabled bit the same as the content attribute, and both reflect and forward the attribute from the setter. That means that setAttribute does something different than setting `disabled`, which means that you can get into all sorts of funny states when reloading the sheet... So I rather not do that. Differential Revision: https://phabricator.services.mozilla.com/D26573 UltraBlame original commit: ef991fe893a435f4416bde5f518135655a9740e1
…reflect that attribute. r=bzbarsky ...instead of forwarding to the sheet like HTMLStyleElement does. I've proposed this behavior in: whatwg/html#3840 (comment) I think this is one of the sane behaviors we can have, what Blink / WebKit do makes no sense to me. Alternative potentially-sane behavior is making the initial value of the stylesheet's disabled bit the same as the content attribute, and both reflect and forward the attribute from the setter. That means that setAttribute does something different than setting `disabled`, which means that you can get into all sorts of funny states when reloading the sheet... So I rather not do that. Differential Revision: https://phabricator.services.mozilla.com/D26573 UltraBlame original commit: edbf9758ea946df5063e84c397c43208c555fd74
|
also, would be really nice. |
|
@a2sheppy apologies for the delay here, there's still a couple things to sort out, but I hope to make some progress again on this. @Zibri that would be a new feature that's probably best filed as a new issue. Looking at the tests, currently Chrome/Safari fail half of 001 and all of 002. Chrome also fails 007. The tests are located at css/cssom/HTMLLinkElement-disabled* in WPT. In particular Chrome/Safari don't null out @emilio https://drafts.csswg.org/cssom/#css-style-sheets doesn't currently say that owner node is supposed to be null when the style sheet is disabled. @tkent-google do you agree 007 is a bug in Chrome? |
Yeah but this makes a disabled link not be the same as a disabled sheet. |
I delegate to @mfreed7. I'm not responsible for HTML any longer. |
|
@annevk when the owner is disabled there's no stylesheet associated to the owner so there's no stylesheet to return null from anywhere. |
|
Sorry for the delay responding to this one. Generally, this sounds like a reasonable suggestion, and improving interop (and sanity) here sounds like a good thing. I'm a bit concerned about compat, given the longstanding weird behavior in Webkit/Blink. I understand that the interop issues might mean the compat issues aren't so bad, but probably not zero either. Does the current Gecko behavior match #4519? |
|
Yes, Gecko behavior is tested in these WPTs. As you may note, Safari and Chrome are pretty close to green in those tests. |
|
Thanks. I have a patch ready that implements the new behavior in Chromium, which I'll try to land soon in M85 and see if there are any reported compat bugs. Thanks for writing the tests for this - made implementing it easy. |
|
Hooraaay! Shall we hold off on landing #4519 until Chromium gets some compat data? Or should we land it, since the current spec doesn't have |
|
Yay! Thanks for doing this Mason! |
Either way is ok by me, but I suppose I'd prefer that it lands. As you mentioned, there's no spec and mixed implementations at the moment, so it seems better to land something and then tweak it if necessary. But clearly, it's up to you. |
Tests: web-platform-tests/wpt#22645. Fixes #3840 and fixes #1081.
Previously, Chromium had intermittent behavior with respect to the "disabled" attribute: - <link id=foo rel="stylesheet" disabled> would not show up in document.styleSheets. - foo.disabled=false; foo.disabled=true; would cause it to appear (and remain) in document.styleSheets. - <link rel="alternate stylesheet"> cannot be enabled, except by disabling and re-enabling it. - When disabled, link.ownerNode was not null The above issues are being resolved with this CL. See the spec discussion and PR here: whatwg/html#3840 whatwg/html#4519 And the ChromeStatus and I2S here: https://chromestatus.com/feature/5110851973414912 https://groups.google.com/a/chromium.org/d/msg/blink-dev/3izM5vVo98U/iDLjz_TwAgAJ= WPT tests are located here: https://wpt.fyi/results/css/cssom?label=master&label=experimental&aligned&q=htmllinkelement-disabled and they show that Firefox already implements the new behavior, while WebKit does not. With this CL, all 7 tests are now green. Bug: 695984, 1087043 Change-Id: Ic6d2c8e901a72885b9f4806c616adb95e019cf09 Cq-Do-Not-Cancel-Tryjobs: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216701 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Mason Freed <masonfreed@chromium.org> Cr-Commit-Position: refs/heads/master@{#774658}
Consider the following test-case:
Current browser behavior is:
<link>appears indocument.styleSheets.document.styleSheets.<link>appears indocument.styleSheets.As far as I can tell, the specified behavior is Firefox's. But that being said, Firefox supports doing
link.disabled = trueand toggling thedisabledflag on the stylesheet, even though it's not in the webidl.Should the
disabledattribute affect whether the sheet is enabled or not? If not, could we get that clarified?Should
disabledbe inHTMLLinkElements IDL?'disabled' in HTMLLinkElement.prototypereturns true everywhere.Should a
disabledsheet appear indocument.styleSheets? Blink and WebKit don't remove it if you toggle the attribute, which looks definitely like a bug to me.I think I'd expect either Firefox's behavior (
disabledattribute has no effect,disabledvia the IDL tweaks the disabled flag of the sheet (effectively forwards setting and getting to thesheet), or Edge's behavior, where thedisabledattribute presumably reflectssheet.disabled(though note I haven't checked this in practice, chances are thatlink.sheet.disabled = truedoesn't affectlink.hasAttribute("disabled")) and stuff like that.cc @bzbarsky @annevk @FremyCompany @lilles @ericwilligers
The text was updated successfully, but these errors were encountered: