1408670 - Bug 1399429 should have used PBU.isContentWindowPrivate instead of PBU.isBrowserPrivate

Status: RESOLVED FIXED

(Firefox :: Private Browsing, enhancement)

Description

[Ian Moody [:Kwan] (UTC+1)]

I misread the code and used the wrong function in bug 1399429. It works fine
(and does the exact same thing) but causes console spam because of [1], and
won't work fine in future when that stops being just a warning.
See bug 1069059 

[1] https://searchfox.org/mozilla-central/rev/1c1a5cef772214e9cff487040ac3068d56e96cc6/toolkit/modules/PrivateBrowsingUtils.jsm#23

Comment 1

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Use PBU.isContentWindowPrivate instead of PBU.isBrowserPrivate in ContextMenu.jsm. r?

Follow up correction to bug 1399429.

MozReview-Commit-ID: JMoTVbc3SAx

Comment 2

[Gian-Carlo Pascutto [:gcp]]

Ian, I think you forgot to flag a reviewer.

Comment 3

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1408670 - Use PBU.isContentWindowPrivate instead of PBU.isBrowserPrivate in ContextMenu.jsm.

Follow up correction to bug 1399429.  isBrowserPrivate causes console spam.

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

Comment 4

[Ian Moody [:Kwan] (UTC+1)]

Comment on attachment 8918571 [details] [diff] [review]
Use PBU.isContentWindowPrivate instead of PBU.isBrowserPrivate in ContextMenu.jsm. r?

Hey Felipe, sorry for the churn, I only later realised isBrowserPrivate causes console spam (and might in the future break).

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8919243 [details]
Bug 1408670 - Use PBU.isContentWindowPrivate instead of PBU.isBrowserPrivate in ContextMenu.jsm.

https://reviewboard.mozilla.org/r/190142/#review195478

I wonder if it's better to move this to the original bug due to the need for the uplift request..

Comment 6

[Ian Moody [:Kwan] (UTC+1)]

Sheriff/c-n elf: No try run since this only really changes from approx.
functionA() {warnInTerminal(); functionBar()}
to
functionB() {functionBar()}
Feel free to ask me for one if you'd prefer.

(In reply to :Felipe Gomes (needinfo me!) from comment #5)
> Comment on attachment 8919243 [details]
> Bug 1408670 - Use PBU.isContentWindowPrivate instead of PBU.isBrowserPrivate
> in ContextMenu.jsm.
> 
> https://reviewboard.mozilla.org/r/190142/#review195478
> 
> I wonder if it's better to move this to the original bug due to the need for
> the uplift request..

Thanks for the quick review!
I was just going to add a branch patch to bug 1399429 with these changes folded in.

Comment 7

[:Felipe Gomes (needinfo for replies!)]

(In reply to Ian Moody [:Kwan] from comment #6)
> Thanks for the quick review!
> I was just going to add a branch patch to bug 1399429 with these changes
> folded in.

Great, works for me!

Comment 8

[Ian Moody [:Kwan] (UTC+1)]

Comment on attachment 8919243 [details]
Bug 1408670 - Use PBU.isContentWindowPrivate instead of PBU.isBrowserPrivate in ContextMenu.jsm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190142/diff/1-2/

Comment 9

[Ian Moody [:Kwan] (UTC+1)]

Okay, actually doing the right thing in the code now, with a try run to prove it:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b369d636d592c1acd55d1524ef041db97fb3172e
(you can see the previous one blowing up try on reviewboard)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6af3f537a063
Use PBU.isContentWindowPrivate instead of PBU.isBrowserPrivate in ContextMenu.jsm. r=Felipe

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6af3f537a063

Comment 12

[Ian Moody [:Kwan] (UTC+1)]

The fix in this was rolled up into the uplift in bug 1399429, so 57 was never actually affected.