Closed Bug 856322 Opened 11 years ago Closed 11 years ago

Private Browsing changes for browser feed preview

Categories

(SeaMonkey :: Feed Discovery and Preview, defect)

defect
Not set
normal

Tracking

(seamonkey2.17 unaffected, seamonkey2.18 fixed, seamonkey2.19 fixed, seamonkey2.20 fixed)

RESOLVED FIXED
seamonkey2.20
Tracking Status
seamonkey2.17 --- unaffected
seamonkey2.18 --- fixed
seamonkey2.19 --- fixed
seamonkey2.20 --- fixed

People

(Reporter: philip.chee, Assigned: philip.chee)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Browser Feed Preview needs to be aware of Private Browsing. Patch coming up.
> -function OpenBrowserWindow()
> +function OpenBrowserWindow(aOptions)
In order not to diverge our PB test too much from the Firefox version I've added a private browsing argument to OpenBrowserWindow()

> +    var notificationBox = getNotificationBox(aContentWindow);
> +    var win = notificationBox.ownerDocument.defaultView;
We can get the chrome window from the notificationBox so we don't need to use all those helper functions like Firefox does.

> -    classID: WCCR_CLASSID,
> -    contractID: WCCR_CONTRACTID,
> +      classID: WCCR_CLASSID,
> +      contractID: WCCR_CONTRACTID,
Remove some stray \tabs.

> -#include "prmem.h"
> -
Useless include, see Bug 798595.

> --- /dev/null
> +++ b/suite/browser/test/browser/browser_privatebrowsing_protocolhandler.js

To run this test:

$ TEST_PATH=suite/browser/test/browser_privatebrowsing_protocolhandler.js  pymake -C ../objdir-sm/ mochitest-browser-chrome

INFO TEST-START | Shutdown
Browser Chrome Test Summary
        Passed: 2
        Failed: 0
        Todo: 0
Attachment #731486 - Flags: review?(neil)
Component: UI Design → Feed Discovery and Preview
Comment on attachment 731486 [details] [diff] [review]
Patch v1.0 Add PB awareness to feed preview.

>+function whenNewWindowLoaded(aOptions, aCallback) {
>+  let win = OpenBrowserWindow(aOptions);
This is teh suck. Just open the browser window directly.
var { private } = aOptions;
var features = private ? "private,chrome,all,dialog=no" : "non-private,chrome,all,dialog=no";
var win = window.openDialog(getBrowserURL(), "_blank", features, "about:blank");

>+    var flags = isPB ? this._faviconService.FAVICON_LOAD_PRIVATE :
>+                       this._faviconService.FAVICON_LOAD_NON_PRIVATE;
[I guess I need to fix up tabbrowser.xml ...]

>+      function (aURI, aDataLen, aData, aMimeType) {
Nit: no space before (?

>+    var isPB = win.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
>+                  .getInterface(Components.interfaces.nsIWebNavigation)
>+                  .QueryInterface(Components.interfaces.nsIDocShell)
>+                  .QueryInterface(Components.interfaces.nsILoadContext)
>+                  .usePrivateBrowsing;
IIRC you can test the private browsing state of the content window to avoid having to do it on the chrome window.
Attachment #731486 - Flags: review?(neil) → review-
> This is teh suck. Just open the browser window directly.
> var { private } = aOptions;
> var features = private ? "private,chrome,all,dialog=no" : "non-private,chrome,all,dialog=no";
> var win = window.openDialog(getBrowserURL(), "_blank", features, "about:blank");
Fixed.

>>+      function (aURI, aDataLen, aData, aMimeType) {
> Nit: no space before (?
Fixed.

>>+    var isPB = win.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
>>+                  .getInterface(Components.interfaces.nsIWebNavigation)
>>+                  .QueryInterface(Components.interfaces.nsIDocShell)
>>+                  .QueryInterface(Components.interfaces.nsILoadContext)
>>+                  .usePrivateBrowsing;
> IIRC you can test the private browsing state of the content window to avoid
> having to do it on the chrome window.
Fixed.
Attachment #731486 - Attachment is obsolete: true
Attachment #735813 - Flags: review?(neil)
Attachment #735813 - Flags: review?(neil) → review+
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/0d3770a601e5
Whiteboard: [leave open for comm-aurora and comm-beta]
Target Milestone: --- → seamonkey2.20
Comment on attachment 735813 [details] [diff] [review]
Patch v1.1 fix review issues.

[Approval Request Comment]
Regression caused by (bug #): Not a regression. 
User impact if declined: Potential information leak
Testing completed (on m-c, etc.): Baked on comm-central without any problems detected.
Risk to taking this patch (and alternatives if risky): Low to none.
String changes made by this patch: None.
Attachment #735813 - Flags: approval-comm-beta?
Attachment #735813 - Flags: approval-comm-aurora?
Attachment #735813 - Flags: approval-comm-beta?
Attachment #735813 - Flags: approval-comm-beta+
Attachment #735813 - Flags: approval-comm-aurora?
Attachment #735813 - Flags: approval-comm-aurora+
Pushed to branches:
http://hg.mozilla.org/releases/comm-aurora/rev/da623ec45a03
http://hg.mozilla.org/releases/comm-beta/rev/ca4626be38b7
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [leave open for comm-aurora and comm-beta]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: