Get rid of nsIHTMLEditor.setDocumentTitle()

RESOLVED FIXED in Firefox 53

Status

()

defect
P3
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: masayuki, Unassigned)

Tracking

Trunk
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(1 attachment)

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).
Flags: needinfo?(neil)
Flags: needinfo?(jorgk)
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.
Flags: needinfo?(neil)
Flags: needinfo?(jorgk)
Flags: needinfo?(iann_bugzilla)
(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.
(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.
Blocks: 1313033
As all is left is TB use, clearing NI
Flags: needinfo?(iann_bugzilla)
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.
(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.
(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.
Flags: needinfo?(daniel)
(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.
Flags: needinfo?(daniel)
Priority: -- → P3
(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 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.
Attachment #8821867 - Flags: review?(bugs) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/032db7cba775
Get rid of nsIHTMLEditor::SetDocumentTitle() and mozilla::SetDocumentTitleTransaction r=smaug
https://hg.mozilla.org/mozilla-central/rev/032db7cba775
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.