Closed Bug 838969 Opened 7 years ago Closed 7 years ago

mozSocial injection incorrectly guarded for private browsing

Categories

(Firefox Graveyard :: SocialAPI, defect)

defect
Not set

Tracking

(firefox20+ fixed)

RESOLVED FIXED
Firefox 21
Tracking Status
firefox20 + fixed

People

(Reporter: markh, Assigned: markh)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files)

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.
Attached patch Fix for trunkSplinter Review
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)
Attached patch Fix for auroraSplinter Review
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+
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+
https://hg.mozilla.org/mozilla-central/rev/15bc5e66f07e
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Whiteboard: [qa-]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.