Closed Bug 1426223 Opened 6 years ago Closed 6 years ago

stylo: Remove Stylo domain blocklist code

Categories

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

57 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: cpeterson, Assigned: chenpighead)

References

Details

Attachments

(2 files)

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.
Thanks for finishing this up Jeremy! :)
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.
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 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 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)
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 :)
https://hg.mozilla.org/mozilla-central/rev/4084a3aa5b5e
https://hg.mozilla.org/mozilla-central/rev/a89a2bef8bd5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: