Closed Bug 1449522 Opened 3 years ago Closed 5 months ago

Remove nsIEditorStyleSheets

Categories

(Core :: DOM: Editor, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: jwatt, Assigned: emilio)

References

Details

Attachments

(4 files)

Follow-up from bug 1449321 comment 3. It looks like nsIEditorStyleSheets code is unused.
Priority: -- → P3
Depends on: 1470361
Blocks: 1471111
No longer blocks: 1471111
Depends on: 1471111

Just looked, these can't work. Also comment 1 is an XBL binding so glazou would have worse problems than this.

Assignee: nobody → emilio

Users have much better, easier alternatives, like
DOMWindowUtils.{loadSheetUsingURIString,removeSheet}, which we use to
replace the only caller that exists in mozilla-central (the editor
element, which loads EditorOverride.css).

This allows to clean up the style system and editor. There are other
callers in comm-central, but it seems they can switch to DOMWindowUtils
trivially, as the DOMWindowUtils APIs also use the system principal and
thus they can load any URL.

I'll make sure to give them some time with the migration and/or help
out of course.

Depends on D71262

Should also have no behavior change.

After the previous patch we don't have sheets associated with a document but
not owned by it, so take advantage of that.

Depends on D71263

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

Just looked, these can't work. Also comment 1 is an XBL binding so glazou would have worse problems than this.

Update: They do work ;)

Magnus, this will affect some Thunderbird callers that are using the existing API from JS. Seems not many:

The alternatives are straight-forward:

  • addOverrideStyleSheet(uri) -> windowUtils.loadSheetUsingURIString(uri, windowUtils.AGENT_SHEET). Both do sync loads and they should load any uri.

  • removeOverrideStyleSheet(uri) -> windowUtils.removeSheet(uri, windowUtils.AGENT_SHEET)`

  • enableStyleSheet(uri, enable) -> (enable ? windowUtils.loadStyleSheetUsingURIString : windowUtils.removeSheet)(uri, windowUtils.AGENT_SHEET).

Let me know if you hit any snags or I can help to do this. I'm happy to wait for a bit until this is done if you need too of course.

Flags: needinfo?(mkmelin+mozilla)

Thanks for the heads up. Here is a patch adjusting the Thunderbird parts. (editor/ is abandoned). Just had to be careful which window's windowUtils to use. Seems to work for me. Looks good?

Flags: needinfo?(mkmelin+mozilla)
Comment on attachment 9141255 [details] [diff] [review]
bug1449522_rm_cc_nsIEditorStyleSheets.patch

Review of attachment 9141255 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, this looks good! Just some minor nits.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +3585,5 @@
>    observe(aSubject, aTopic, aData) {
>      if (aTopic == "obs_documentCreated") {
>        var editor = GetCurrentEditor();
>        if (editor && GetCurrentCommandManager() == aSubject) {
> +        // We use loadSheetUsingURIString so that we geta synchronous load,

s/geta/get a

@@ +3589,5 @@
> +        // We use loadSheetUsingURIString so that we geta synchronous load,
> +        // rather than having a late-finishing async load mark our editor as
> +        // modified when the user hasn't typed anything yet, but that means the
> +        // sheet must not @import slow things, especially not over the network.
> +        let domWindowUtils = document.querySelector("editor").contentWindow

I think instead of querySelector() you could use `GetCurrentEditorElement()` which seems more in line with `GetCurrentEditor()`. And maybe do both at the same time from InitEditor() if you're on the mood, since this is its only caller.

::: mail/components/compose/content/editor.js
@@ +182,5 @@
>          if (!("InsertCharWindow" in window)) {
>            window.InsertCharWindow = null;
>          }
>  
> +        let domWindowUtils = document.querySelector("editor").contentWindow

Same here.
Attachment #9141255 - Flags: review+
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c14d6b8b6ad
Remove nsIEditorStyleSheets. r=masayuki,m_kato
https://hg.mozilla.org/integration/autoland/rev/537d098b45cb
Clean up stylesheet association modes. r=nordzilla
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8d00b26084f9
Cleanup some parent walks in StyleSheet code. r=nordzilla
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/2a1b8d8994d0
remove Thunderbird nsIEditorStyleSheets usage. r=emilio
You need to log in before you can comment on or make changes to this bug.