Closed Bug 1042991 Opened 6 years ago Closed 6 years ago

history breakage in share panel

Categories

(Firefox Graveyard :: SocialAPI, defect)

32 Branch
x86
macOS
defect
Not set

Tracking

(firefox31 wontfix, firefox32 fixed, firefox33 fixed, firefox34 fixed)

RESOLVED FIXED
Firefox 34
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
firefox34 --- fixed

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch support history in share panel (obsolete) — Splinter Review
Facebook share panel recently broke, the panel no longer closes when the user cancels or completes a share.  Examination of the facebook code shows that history.back() is called prior to window.close().  Since the share panel uses an iframe, the history object is not available, an exception is thrown, and window.close is never called.

The intention with the share panel is that existing share dialogs provided by many sites will work unchanged, however, all socialapi panels intentionally used iframe, in part, to avoid history being added to the places database.  For the share panel to work with share dialogs unchanged, we'll need to be sure we support normal web/js APIs that would otherwise be available.

The easy fix is to use the browser element and remove history for that browser when share is done.
Attachment #8461160 - Flags: review?(jaws)
Attached patch support history in share panel (obsolete) — Splinter Review
small fix with working try

https://tbpl.mozilla.org/?tree=Try&rev=633bf2c03b7f
Assignee: nobody → mixedpuppy
Attachment #8461160 - Attachment is obsolete: true
Attachment #8461160 - Flags: review?(jaws)
Attachment #8461616 - Flags: review?(jaws)
Attached patch support history in share panel (obsolete) — Splinter Review
last upload I grabbed an old version of the patch, this has the small fix mentioned and try'd earlier.
Attachment #8461616 - Attachment is obsolete: true
Attachment #8461616 - Flags: review?(bmcbride)
Attachment #8461671 - Flags: review?(bmcbride)
Comment on attachment 8461671 [details] [diff] [review]
support history in share panel

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

r+ for the code, r- for lack of a test.

::: browser/base/content/browser-social.js
@@ +715,5 @@
> +      let purge = iframe.sessionHistory.count;
> +      if (purge > 0)
> +        iframe.sessionHistory.PurgeHistory(purge);
> +    }
> +  

Nit: Trailing whitespace (the horror!)
Attachment #8461671 - Flags: review?(bmcbride) → review-
tests for history support (which broke the tests if the fixes were not applied) as well as testing that global history is not retained (ie. share.html is not in history after tests)

https://tbpl.mozilla.org/?tree=Try&rev=c93a276e54bf
Attachment #8461671 - Attachment is obsolete: true
Attachment #8464350 - Flags: review?(bmcbride)
Attachment #8464350 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/mozilla-central/rev/882720470f4f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Comment on attachment 8464350 [details] [diff] [review]
support history in share panelfixsharehistory

Approval Request Comment
[Feature/regressing bug #]: share feature
[User impact if declined]: facebook share does not appear to work (it no longer closes the panel after sharing or cancelling)
[Describe test coverage new/current, TBPL]: new test coverage for the history api in content
[Risks and why]: low
[String/UUID change made/needed]: none
Attachment #8464350 - Flags: approval-mozilla-beta?
Attachment #8464350 - Flags: approval-mozilla-aurora?
Flags: in-testsuite+
Attachment #8464350 - Flags: approval-mozilla-beta?
Attachment #8464350 - Flags: approval-mozilla-beta+
Attachment #8464350 - Flags: approval-mozilla-aurora?
Attachment #8464350 - Flags: approval-mozilla-aurora+
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.