1487856 - "No Style" function doesn't disable HTML "style" attributes

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[astian]

The feature to disable author/document level styling (View > Page Style > No
Style) no longer disables styles embedded in HTML "style" attributes.  This
works in Firefox 52 ESR but is broken in Firefox 60 ESR (also in Firefox 61).

Steps to reproduce:

  1. Disable javascript (use an extension or toggle javascript.enabled).
  2. Visit <https://web.archive.org/https://kernel.org/>.
  3. Activate "View > Page Style > No Style".

Actual results:

  Though linked and embedded stylesheets seem to be correctly disabled, styles
  specified in HTML "style" attributes are not.

  For these STR, notice that element #wm-ipp remains out of the document
  layout because it contains a 'style="display:none;..."' attribute.  Manually
  disabling that (for example, in the Inspector) will show it.

Expected results:

  Firefox should disable all author/document level styling, including "style"
  HTML attributes, like it used to in 52 ESR.

As you can see this regression renders parts of some pages completely
inaccessible.

Notice: The builds of Firefox I tested were the ones shipped by Debian.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

You're right, this is a Stylo regression. Should be pretty easy to fix.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Make the author styles disabled stuff actually disable style attribute, animations, and XBL rules.

This also removes one of my FIXMEs from when I was looking at this code.

We don't seem to have a pre-existing test for this feature, sigh. I'll try to
write one if I have cycles for it...

Note that it not applying XBL rules is a feature, given the current state of
affairs. Video controls and such are right now unusable with no styles enabled.

Comment 3

[Cameron McCormack (:heycam)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)
> Note that it not applying XBL rules is a feature, given the current state of
> affairs. Video controls and such are right now unusable with no styles
> enabled.

Is there a bug on file for this?

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 9005716 [details]
Make the author styles disabled stuff actually disable style attribute, animations, and XBL rules.

Cameron McCormack (:heycam) has approved the revision.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5aa8199bc161
Make the author styles disabled stuff actually disable style attribute, animations, and XBL rules. r=heycam

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/5aa8199bc161

Comment 7

[astian]

This fixes a regression, please push to ESR.

Comment 8

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 63.0a1 (2018.08.31) on Windows 10 x64. 
Verified on ESR 52.9.0 it works.
I can confirm this issue is fixed, I verified using Firefox 63.0b5 on Ubuntu 16.04 x64, Windows 10 x64 and Mac OS X 10.14.

Comment 9

[astian]

See comment #7.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

I'd need to rebase the patch, and I'd be happy to if needed.

Though but I honestly don't think it's a particularly good candidate for ESR uplift. I don't think it's critical and this has been the only report of it since FF 57 (which is when this broke).

But you're welcome to make a case if you think it is, and a request toggling the attachment details and setting the approval-mozilla-esr flag to '?' so that the people that decide whether to uplift this or not have the right information.

Thanks for the report.