Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify ChromeActions.isInPrivateBrowsing to not rely on the global PB service #2498

Merged
merged 1 commit into from May 30, 2013

Conversation

ehsan
Copy link
Contributor

@ehsan ehsan commented Dec 21, 2012

This addresses the comments that I had on #2481. PrivateBrowsingUtils.isWindowPrivate works fine without throwing in all Firefox versions now, and there is no need for the nsIPrivateBrowsingService fallback any more. Also, it doesn't make sense to cache the result here since isWindowPrivate is not an expensive call. This patch heavily simplifies isInPrivateBrowsing.

@ehsan
Copy link
Contributor Author

ehsan commented Dec 21, 2012

/cc @brendandahl @yurydelendik

@brendandahl
Copy link
Contributor

We have many users on the extension on various versions of Firefox so we can't accept something that won't work for older versions of FF.

@ehsan
Copy link
Contributor Author

ehsan commented Dec 22, 2012

This will work on Firefox 13 and newer. Is that not acceptable?

@yurydelendik
Copy link
Contributor

Firefox 10 esr?

@yurydelendik
Copy link
Contributor

@ehsan , I cannot verify that PrivateBrowsingUtils exist on FF13, looks like it was introduced only in Firefox 16 https://bugzilla.mozilla.org/show_bug.cgi?id=769467 .

Has Seamonkey PrivateBrowsingUtils.isWindowPrivate or nsIPrivateBrowsingService ?

@ehsan
Copy link
Contributor Author

ehsan commented Dec 29, 2012

Sorry, yeah you're right, I was confusing it with the usePrivateBrowsing attribute. PrivateBrowsingUtils is available in all Mozilla based applications includling SeaMonkey and Thunderbird. But the ESR10 compatibility is a good point. Perhaps we can revisit this when ESR17 is released, at which point PrivateBrowsingUtils can be used everywhere safely.

@waddlesplash
Copy link
Contributor

Firefox passed v17 two versions ago now. What is the status of this patch?

@ehsan
Copy link
Contributor Author

ehsan commented Mar 7, 2013

I think we can take it now. @yurydelendik ?

@brendandahl
Copy link
Contributor

Seems fine to take now. Can you de-rot it?

@ehsan
Copy link
Contributor Author

ehsan commented Apr 27, 2013

Done!

@vyv03354
Copy link
Contributor

I think em:minVersion should be bumped, too. (17.0 for Firefox, 2.14 for SeaMonkey.)

brendandahl added a commit that referenced this pull request May 30, 2013
Simplify ChromeActions.isInPrivateBrowsing to not rely on the global PB service
@brendandahl brendandahl merged commit 24204cc into mozilla:master May 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants