stylo: Remove Stylo domain blocklist code

RESOLVED FIXED in Firefox 60

Status

()

enhancement
P4
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: cpeterson, Assigned: jeremychen)

Tracking

57 Branch
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

a year ago
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.
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".
Assignee: nobody → jeremychen
Blocks: 1430014
No longer depends on: stylo-everywhere
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 hidden (mozreview-request)
Thanks for finishing this up Jeremy! :)

Comment 7

a year ago
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

a year ago
See also: bug 1434035 (stylo: ship system add-on to disable Stylo for Outlook and Hotmail)
(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.
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).
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

a year ago
mozreview-review
Comment on attachment 8946017 [details]
Bug 1426223 - remove tests for Stylo blocklist mechanism.

https://reviewboard.mozilla.org/r/216088/#review222202
Attachment #8946017 - Flags: review?(xidorn+moz) → review+

Comment 13

a year ago
mozreview-review
Comment on attachment 8946018 [details]
Bug 1426223 - remove Stylo domain blocklist mechanism.

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

Thanks for finishing this.
Attachment #8946018 - Flags: review?(xidorn+moz) → review+
(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.
Keywords: checkin-needed
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

a year ago
Pushed by ecoal95@gmail.com:
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
Keywords: checkin-needed
(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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4084a3aa5b5e
https://hg.mozilla.org/mozilla-central/rev/a89a2bef8bd5
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.