nsNPAPIPlugin check for private browsing value will not work

RESOLVED FIXED in mozilla15

Status

()

Core
Plug-ins
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jdm, Assigned: Ehsan)

Tracking

Trunk
mozilla15
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
>::: dom/plugins/base/nsNPAPIPlugin.cpp
>@@ +2136,5 @@
>>    }
>>  
>>    case NPNVprivateModeBool: {
>> +    nsCOMPtr<nsIDocument> doc = GetDocumentFromNPP(npp);
>> +    nsCOMPtr<nsPIDOMWindow> domwindow = do_QueryInterface(doc);
>
>This will always be null.

Looks like this should be |domwindow = doc->GetWindow()| instead.
Created attachment 618300 [details] [diff] [review]
Patch (v1)
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #618300 - Flags: review?(joshmoz)
(Reporter)

Comment 2

5 years ago
Every other caller of GetDocumentFromNPP null checks, so we should probably do the same.
Created attachment 618516 [details] [diff] [review]
Patch (v2)
Attachment #618300 - Attachment is obsolete: true
Attachment #618300 - Flags: review?(joshmoz)
Attachment #618516 - Flags: review?(joshmoz)

Comment 4

5 years ago
Comment on attachment 618516 [details] [diff] [review]
Patch (v2)

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

Is this something a test should have caught? We do have an NPAPI private mode test.

dom/plugins/test/mochitest/test_privatemode.xul
Attachment #618516 - Flags: review?(joshmoz) → review+
(In reply to Josh Aas (Mozilla Corporation) from comment #4)
> Comment on attachment 618516 [details] [diff] [review]
> Patch (v2)
> 
> Review of attachment 618516 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Is this something a test should have caught? We do have an NPAPI private
> mode test.
> 
> dom/plugins/test/mochitest/test_privatemode.xul

I have no idea how to trigger this code in a test...
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf2eb02383fd
Target Milestone: --- → mozilla15
(Reporter)

Comment 7

5 years ago
The problem existed in the NPN_GetValue code, which is tested by queryPrivateModeState (http://mxr.mozilla.org/mozilla-central/source/dom/plugins/test/mochitest/test_privatemode.xul#38). The test as written would pass, since it tests non-PB with qPMS, then tests that entering PB mode triggers an update message for plugin instances via NPN_SetValue (http://mxr.mozilla.org/mozilla-central/source/dom/plugins/test/mochitest/test_privatemode.xul#59). Since it doesn't compare this against the qPMS value, the test doesn't see anything as being wrong. This check should be added to the test, I think.
(Reporter)

Updated

5 years ago
Flags: in-testsuite?

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/bf2eb02383fd
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Updated

5 years ago
Depends on: 749795
Josh, you said in bug 749795 that you're working on a test for this, right?
(Reporter)

Comment 10

5 years ago
Yes.
(Reporter)

Updated

5 years ago
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.