1312991 - Get rid of nsIHTMLEditor.setDocumentTitle()

Status: RESOLVED FIXED

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

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

If we will stop supporting undoable document title change via a call of nsIHTMLEditor.setDocumentTitle() (bug 1312989), we don't need to keep supporting nsIHTMLEditor.setDocumentTitle() anymore because |document.title = "foo";| is available.

I also want to ask ni? to Thunderbird and SeaMonkey guys if they have some trouble if we'll remove them. Same as bug 1312989, I'd like to remove it in 53 (after next ESR).

Comment 1

[Jorg K (CEST = GMT+2)]

We use editor.setDocumentTitle(title); a few times here
https://dxr.mozilla.org/comm-central/search?q=setDocumentTitle&redirect=false

So instead of
  GetCurrentEditor().setDocumentTitle(title);
we'd have to dig out the document and set its title instead, right?

I haven't seen Neil in ages, so I'm forwarding this to Ian.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

(In reply to Jorg K (GMT+2) from comment #1)
> We use editor.setDocumentTitle(title); a few times here
> https://dxr.mozilla.org/comm-central/search?q=setDocumentTitle&redirect=false
> 
> So instead of
>   GetCurrentEditor().setDocumentTitle(title);
> we'd have to dig out the document and set its title instead, right?

I think that |GetCurrentEditorElement().ownerDocument.title = title;| is enough.

> I haven't seen Neil in ages, so I'm forwarding this to Ian.

Thanks! I referred module owners of SeaMonkey, though.

Comment 3

[Jorg K (CEST = GMT+2)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #2)
> I think that |GetCurrentEditorElement().ownerDocument.title = title;| is
> enough.
That works now, right? So we could make the change now.

TB also has a few of these: this.setDocumentTitle(tab);
https://dxr.mozilla.org/comm-central/rev/7c2e6b4b08219dc2e2c2278d0fc645678166145e/mail/base/content/tabmail.xml#587
but I guess that 'this' is the document.

Comment 4

[Ian Neal]

As all is left is TB use, clearing NI

Comment 5

[Jorg K (CEST = GMT+2)]

We've removed the use of nsIHTMLEditor.setDocumentTitle() in C-C in bug 1313033.

There is a method by the same name in mail/base/content/tabmail.xml
https://dxr.mozilla.org/comm-central/search?q=setDocumentTitle&redirect=false
but that's unrelated.

Comment 6

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to Jorg K (GMT+1) from comment #5)
> We've removed the use of nsIHTMLEditor.setDocumentTitle() in C-C in bug
> 1313033.

BlueGriffon used it.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

(In reply to Daniel Glazman (:glazou) from comment #6)
> (In reply to Jorg K (GMT+1) from comment #5)
> > We've removed the use of nsIHTMLEditor.setDocumentTitle() in C-C in bug
> > 1313033.
> 
> BlueGriffon used it.

Do you disagree with removing nsIHTMLEditor.setDocumentTitle()? As I mentioned in bug 1312989, it doesn't make sense we keep supporting "undoable" document title change.

Comment 8

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #7)

> Do you disagree with removing nsIHTMLEditor.setDocumentTitle()? As I
> mentioned in bug 1312989, it doesn't make sense we keep supporting
> "undoable" document title change.

It's done so no need to argue here, but making it undoable was not
difficult, I would have preferred that way of doing. *Anything*
the user does through UI should be undoable, including when
"Document properties" are tweaked.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

(In reply to Daniel Glazman (:glazou) from comment #8)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #7)
> 
> > Do you disagree with removing nsIHTMLEditor.setDocumentTitle()? As I
> > mentioned in bug 1312989, it doesn't make sense we keep supporting
> > "undoable" document title change.
> 
> It's done so no need to argue here, but making it undoable was not
> difficult, I would have preferred that way of doing. *Anything*
> the user does through UI should be undoable, including when
> "Document properties" are tweaked.

The difficult point of this is, changing document title is not "pure" user operation. I mean that it needs helper UI and such UI tries to modify the document title with JS API. Such change can be done without user operation. So, document title change in undo stack may not be expected by user. So, that's same as other DOM tree changes from JS.

I wonder, editor should be undoable even with changed by JS because modern web apps override default behavior with their script. Although, I have no idea how we should do that.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

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

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Bug 1312991 Get rid of nsIHTMLEditor::SetDocumentTitle() and mozilla::SetDocumentTitleTransaction

Now, nobody (including add-ons) uses nsIHTMLEditor::SetDocumentTitle(), so, we can remove it.

Additionally, mozilla::SetDocumentTitleTransaction is created only when nsIHMLEditor::SetDocumentTitle(), so, we can remove this class too.

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

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8821867 [details]
Bug 1312991 Get rid of nsIHTMLEditor::SetDocumentTitle() and mozilla::SetDocumentTitleTransaction

https://reviewboard.mozilla.org/r/100978/#review101618

This is a type of patch where MozReview fails badly, since it doesn't show the contents of the removed files.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/032db7cba775
Get rid of nsIHTMLEditor::SetDocumentTitle() and mozilla::SetDocumentTitleTransaction r=smaug

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/032db7cba775