1426223 - stylo: Remove Stylo domain blocklist code

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P4)

Description

[Chris Peterson [:cpeterson]]

We added a Stylo domain blocklist in bug 1403077 so we could disable Stylo for individual domains. We haven't needed to disable Stylo for any domains in Firefox 57, so we can eventually remove this blocklist code. This is not a high priority.

Comment 1

[Jeremy Chen ([e-mail])]

Assign this to myself since I'm the one who added this in the first place.

I think we can remove the blocklist code at some time between now and the time when we "remove/disable old style system codes".

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

I think we should remove it now so that we don't need to touch the related code in bug 1430014 for disable the old style system.

Comment 3

[Jeremy Chen ([e-mail])]

Attachment: Bug 1426223 - remove tests for Stylo blocklist mechanism.

In this patch, we remove 3 tests:
1. test for blocked domain
2. test for blocked sub-domain
3. test for non-blocked domain

Two nsIDOMWindowUtils APIs were added for adding/removing a mock domain to the
existing blocklist. They are only used in the Stylo blocklist tests, so we
remove them as well.

Review commit: https://reviewboard.mozilla.org/r/216088/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216088/

Comment 4

[Jeremy Chen ([e-mail])]

Attachment: Bug 1426223 - remove Stylo domain blocklist mechanism.

Review commit: https://reviewboard.mozilla.org/r/216090/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216090/

Comment 5

[Jeremy Chen ([e-mail])]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b664c3a9be251e15e2c197f885339acc9e4219c0

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Thanks for finishing this up Jeremy! :)

Comment 7

[jscher2000]

Sorry, but could I request not rushing this change? 

We have a Stylo issue in Firefox 58-60 on Outlook mail (outlook.live.com and outlook.office365.com). 

Bug 1433591

As long as this blocklist mechanism exists, SuMo could help users enable it manually as a workaround if they do not like the other workarounds suggested.

Comment 8

[jscher2000]

See also: bug 1434035 (stylo: ship system add-on to disable Stylo for Outlook and Hotmail)

Comment 9

[Emilio Cobos Álvarez (:emilio)]

(In reply to jscher2000 from comment #8)
> See also: bug 1434035 (stylo: ship system add-on to disable Stylo for
> Outlook and Hotmail)

I don't think this changes this bug, the code should be removed from trunk, since we plan to remove the old style system anyway, and we should stop maintaining both ASAP.

Comment 10

[Bobby Holley (:bholley)]

Yeah, it seems like we ended up regressing something in the style system in 58, and we happen to be in the position to hotfix that by switching on the old style system for the affected sites. But that shouldn't be a driving factor for maintaining the old style system, which we hope to drop in 60 (this release).

Comment 11

[Xidorn Quan [:xidorn] UTC+10]

I think even if we fail to drop the old style system in 60, we should consider stylo as the only style system we support for now, and the direction should be fixing bugs in that rather than using the blocklist as a workaround.

There have been certain differences between stylo and the old style system, so adding domains into the blocklist also has its own risk of regressing other stuff.

Comment 12

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8946017 [details]
Bug 1426223 - remove tests for Stylo blocklist mechanism.

https://reviewboard.mozilla.org/r/216088/#review222202

Comment 13

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8946018 [details]
Bug 1426223 - remove Stylo domain blocklist mechanism.

https://reviewboard.mozilla.org/r/216090/#review222210

Thanks for finishing this.

Comment 14

[Jeremy Chen ([e-mail])]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #6)
> Thanks for finishing this up Jeremy! :)

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #13)
> Comment on attachment 8946018 [details]
> Bug 1426223 - remove Stylo domain blocklist mechanism.
> 
> https://reviewboard.mozilla.org/r/216090/#review222210
> 
> Thanks for finishing this.

Thank you!!! It's my honor to work with you.

Comment 15

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 6a3ad9c191099795bf6856a680f9f060b41e3753 -d 2573f47a1353: rebasing 444734:6a3ad9c19109 "Bug 1426223 - remove tests for Stylo blocklist mechanism. r=xidorn"
merging dom/base/nsDOMWindowUtils.cpp
merging dom/interfaces/base/nsIDOMWindowUtils.idl
merging layout/base/nsLayoutUtils.cpp
merging layout/base/nsLayoutUtils.h
warning: conflicts while merging dom/interfaces/base/nsIDOMWindowUtils.idl! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4084a3aa5b5e
remove tests for Stylo blocklist mechanism. r=xidorn
https://hg.mozilla.org/integration/autoland/rev/a89a2bef8bd5
remove Stylo domain blocklist mechanism. r=xidorn

Comment 17

[Emilio Cobos Álvarez (:emilio)]

(In reply to Mozilla Autoland from comment #15)
> We're sorry, Autoland could not rebase your commits for you automatically.
> Please manually rebase your commits and try again.
> 
> hg error in cmd: hg rebase -s 6a3ad9c191099795bf6856a680f9f060b41e3753 -d
> 2573f47a1353: rebasing 444734:6a3ad9c19109 "Bug 1426223 - remove tests for
> Stylo blocklist mechanism. r=xidorn"
> merging dom/base/nsDOMWindowUtils.cpp
> merging dom/interfaces/base/nsIDOMWindowUtils.idl
> merging layout/base/nsLayoutUtils.cpp
> merging layout/base/nsLayoutUtils.h
> warning: conflicts while merging dom/interfaces/base/nsIDOMWindowUtils.idl!
> (edit, then use 'hg resolve --mark')
> unresolved conflicts (see hg resolve, then hg rebase --continue)

This conflicted with bug 1433139 and bug 1406274, but the rebase was trivial, so I took the liberty to push it. Thanks a lot again Jeremy, it's a pleasure to have worked with you :)

Comment 18

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4084a3aa5b5e
https://hg.mozilla.org/mozilla-central/rev/a89a2bef8bd5