Closed Bug 1576505 Opened 2 years ago Closed 2 years ago

Remove use of contentDocumentAsCPOW which was removed from M-C in bug 1127393

Categories

(Thunderbird :: General, task)

task
Not set
normal

Tracking

(thunderbird_esr6869+ fixed, thunderbird69 fixed, thunderbird70 fixed)

RESOLVED FIXED
Thunderbird 70.0
Tracking Status
thunderbird_esr68 69+ fixed
thunderbird69 --- fixed
thunderbird70 --- fixed

People

(Reporter: jorgk-bmo, Assigned: frg)

Details

Attachments

(1 file)

Remove use of contentDocumentAsCPOW which was removed from M-C in bug 1127393:
https://hg.mozilla.org/mozilla-central/rev/b94a2aa50ef2

We use it here:
https://searchfox.org/comm-central/search?q=contentDocumentAsCPOW&case=false&regexp=false&path=
One is a comment and ViewSourceSavePage() appears to be dead code.

P.S.: I got the tip from FRG.

I only have a TB 60 here. This is probably triggered/used there in the source view of an email when you use "Save Page As… "

It definitely used when you use save in view message source. Tried with esr60 where it still works. But you can provide an undefined variable and the InternalSave seems not to care. Falls back to gPrivate (not provided in parameter 13) and if this one is undefined too seems to use null.

If you think this is the right patch please assign me. Otherwise I wouldn't know what to replace it with.

Attachment #9088240 - Flags: review?(jorgk)
Comment on attachment 9088240 [details] [diff] [review]
1576505-contentDocumentAsCPOW.patch

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

::: common/src/viewSource.js
@@ +760,5 @@
>  // Strips the |view-source:| for internalSave()
>  function ViewSourceSavePage() {
>    internalSave(gBrowser.currentURI.spec.replace(/^view-source:/i, ""),
>                 null, null, null, null, null, "SaveLinkTitle",
> +               null, null, gBrowser.contentDocument, null,

Wouldn't it rather be:
https://hg.mozilla.org/mozilla-central/rev/b94a2aa50ef2#l2.13
gBrowser.contentWindow?

We want a window, not a document, or am I missing something?

We want a window, not a document, or am I missing something?

Looking at the esr60 code where this is still in my vote is document. We don't have a remote browser because no e10s. So it is just the contentDocument from browser.xml. Changed it to contentDocument and tried in SeaMonkey 2.57 which has a private window mode. "Save as" did the right thing and didn't add it to the download history so it must have picked the right document. But don't call me an expert here :)

  • @param aInitiatingDocument [optional]
  •    The document from which the save was initiated.
    
  •    If this is omitted then aIsContentWindowPrivate has to be provided.
    

Bug 1051017 added it:
https://hg.mozilla.org/releases/mozilla-esr60/annotate/0f28c3a1e6a783a09e5550973b5554ac53b3e690/toolkit/content/widgets/browser.xml#l513

Comment on attachment 9088240 [details] [diff] [review]
1576505-contentDocumentAsCPOW.patch

You convinced me, thanks.
Attachment #9088240 - Flags: review?(jorgk)
Attachment #9088240 - Flags: review+
Attachment #9088240 - Flags: approval-comm-esr68+
Attachment #9088240 - Flags: approval-comm-beta+
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 70.0

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/bf8d4215b3a0
Replace obsolete contentDocumentAsCPOW. r=jorgk

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED

Thanks, FRG. New contributor to the project? ;-) - Oh, no, this is shared code.

Hmm, "Save Page As… " doesn't work in TB 68, you get:

ReferenceError: can't access lexical declaration `SAVEMODE_FILEONLY' before initialization contentAreaUtils.js:1364:5
GetSaveModeForContentType chrome://global/content/contentAreaUtils.js:1364
internalSave chrome://global/content/contentAreaUtils.js:469
ViewSourceSavePage chrome://messenger/content/viewSource.js:756
oncommand chrome://messenger/content/viewSource.xul:1

Did you try it?

Flags: needinfo?(frgrahl)

Filed bug 1577289.

No I tried only on 60. Other than keeping SM compiled I currently don't do anything on comm.central. Lost all interest there.

This does sound like a different bug.

FRG

Flags: needinfo?(frgrahl)

Might be related:
// We can only save a complete page if we have a loaded document,
// and it's not a CPOW -- nsWebBrowserPersist needs a real document.

https://searchfox.org/comm-central/source/mozilla/toolkit/content/contentAreaUtils.js#1359

If this no longer works in non e10s you might need to pass parameter 13. Probably always false for TB.

You need to log in before you can comment on or make changes to this bug.