mozSocial injection incorrectly guarded for private browsing

RESOLVED FIXED in Firefox 20

Status

()

RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: markh, Assigned: markh)

Tracking

Trunk
Firefox 21
Points:
---

Firefox Tracking Flags

(firefox20+ fixed)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
Bug 808215 introduced the following blocks to MozSocialAPI.jsm:

+#ifndef MOZ_PER_WINDOW_PRIVATE_BROWSING
+XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils", "resource://gre/modules/PrivateBrowsingUtils.jsm");
+#endif

...
-    if (!window)
+    if (!window
+#ifndef MOZ_PER_WINDOW_PRIVATE_BROWSING
+        || !PrivateBrowsingUtils.isWindowPrivate(window)
+#endif
+       )

These were wrong - the last change should have read:

+#ifdef MOZ_PER_WINDOW_PRIVATE_BROWSING
+        || PrivateBrowsingUtils.isWindowPrivate(window)
+#endif

(Note the negation of *both* the #if and the result of isWindowPrivate) as the intention was to continue injecting in non-per-window builds, but prevent injecting in per-window builds.  There is also one further similar problem later in the file relating to chat windows.

The end result of this is that:

* trunk has had the check removed when all the MOZ_PER_WINDOW_PRIVATE_BROWSING was ripped out.  While this is benign (as none of the social UI elements are created anyway), the check should be re-added to trunk for the sake of being extra-careful.

* Builds on the birch tree (basically Aurora but without per-window PB) are currently broken due to most tests failing - we do not inject when we should.

Attaching 2 different patches to address this.
(Assignee)

Comment 1

6 years ago
Created attachment 711162 [details] [diff] [review]
Fix for trunk

This is the fix for trunk, where all #if[n]def statements for private browsing have been removed.
Assignee: nobody → mhammond
Attachment #711162 - Flags: review?(gavin.sharp)
(Assignee)

Comment 2

6 years ago
Created attachment 711164 [details] [diff] [review]
Fix for aurora

This is the fix for Aurora and resolves the test failures on the birch branch
Attachment #711164 - Flags: review?(gavin.sharp)
Attachment #711162 - Flags: review?(gavin.sharp) → review+
Attachment #711164 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 3

6 years ago
Comment on attachment 711164 [details] [diff] [review]
Fix for aurora

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 808215
User impact if declined: Social features broken in non-perwindow PB builds
Testing completed (on m-c, etc.): this is an aurora only patch
Risk to taking this patch (and alternatives if risky): Risks are small and limited to social.  The code only has an impact if per-window PB is disabled later in the release cycle (ie, currently the problem this patch fixes can only be observed on the "birch" branch.
String or UUID changes made by this patch: None
Attachment #711164 - Flags: approval-mozilla-aurora?
Attachment #711164 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 4

6 years ago
Pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/3f1a48c48015
status-firefox20: affected → fixed
https://hg.mozilla.org/mozilla-central/rev/15bc5e66f07e
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21

Updated

6 years ago
tracking-firefox20: ? → +
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.