1449522 - Remove nsIEditorStyleSheets

Status: RESOLVED FIXED

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

Description

[Jonathan Watt [:jwatt]]

Follow-up from bug 1449321 comment 3. It looks like nsIEditorStyleSheets code is unused.

Comment 1

[Makoto Kato [:m_kato]]

bluegriffon and c-c still use this.

https://github.com/therealglazou/bluegriffon/search?utf8=%E2%9C%93&q=nsIEditorStyleSheets+&type= 
https://searchfox.org/comm-central/source/mail/components/compose/content/MsgComposeCommands.js#3038

Comment 2

[Emilio Cobos Álvarez (:emilio)]

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

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449522 - Remove nsIEditorStyleSheets. r=masayuki,m_kato

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

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449522 - Clean up stylesheet association modes. r=nordzilla,#style

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

Comment 5

[Emilio Cobos Álvarez (:emilio)]

(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:

• https://searchfox.org/comm-central/search?q=addOverrideStyleSheet&case=false&regexp=false&path=

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.

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: bug1449522_rm_cc_nsIEditorStyleSheets.patch

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?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

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.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449522 - Cleanup some parent walks in StyleSheet code. r=nordzilla,#style

Depends on D71264

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d00b26084f9
Cleanup some parent walks in StyleSheet code. r=nordzilla

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/5c14d6b8b6ad
https://hg.mozilla.org/mozilla-central/rev/537d098b45cb
https://hg.mozilla.org/mozilla-central/rev/8d00b26084f9

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2a1b8d8994d0
remove Thunderbird nsIEditorStyleSheets usage. r=emilio