984628 - share button no longer works

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Shane Caraveo (:mixedpuppy)]

I've seen various broken issues with the share button in fx30, unable to repro in fx29.  Initially noticed on fx-team build from today on linux, reproduced on osx.

- after removing share providers button is still visible
- after adding first share provider button is not visible
- share button became visible after installing 3 share providers

str

all str using http://mixedpuppy.github.io/socialapi-directory/

- start with no share providers
- install facebook share
X share button becomes visible but will not open on click
- remove facebook share
X button remains visible, should not be
- add facebook share again
X share button becomes visible but will not open on click
- add g+ mockup
X share button remains visible but will not open on click
- add demo provider
X share button remains visible and CAN be opened on click

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: fix button states

regression caused by bug 894806, r? first come first serve.

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8393719 [details] [diff] [review]
fix button states

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

Seeing we regressed this I think we should consider adding a test for the regressed behaviour.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: fix button states

now with tests!

I ran the tests without the fixes to ensure they properly failed.  One of the reasons these passed was that the share tests included a sidebar, which bypassed one of the primary bugs.  Now that we have some share-only providers I started hitting that bug.

I'm pretty sure this is fx30-only, but am going to test other versions tomorrow to make sure.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8393895 [details] [diff] [review]
fix button states

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

Thanks!

::: browser/base/content/test/social/browser_share.js
@@ +14,4 @@
>  function test() {
>    waitForExplicitFinish();
>  
> +  //runSocialTestWithProvider(manifest, function (finishcb) {

just kill these commented lines

@@ +130,5 @@
> +        checkSocialUI();
> +        // share should not be enabled since we only have about:blank page
> +        let shareButton = SocialShare.shareButton;
> +        is(shareButton.disabled, false, "share button is enabled");
> +        // verify the attribute ifor proper css

same typo here

::: browser/base/content/test/social/browser_social_marks.js
@@ +82,5 @@
> +
> +      // bypass widget go directly to dom, check attribute states
> +      let button = document.getElementById(id);
> +      is(button.disabled, true, "mark button is disabled");
> +      // verify the attribute ifor proper css

typo here

Comment 5

[Shane Caraveo (:mixedpuppy)]

Attachment: fix button states

review comments addressed.

https://tbpl.mozilla.org/?tree=Try&rev=c322276ccbc5

Comment 6

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/integration/fx-team/rev/5d6187699584

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5d6187699584

Comment 8

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8393915 [details] [diff] [review]
fix button states

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 894806
User impact if declined: share button will not open when clicked on
Testing completed (on m-c, etc.): fx-team, m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 9

[Shane Caraveo (:mixedpuppy)]

Beta is also affected by this to a lesser extent.  Whereas opening the share panel was broken in addition to ui glitches the patch fixed, for beta it is limited to the ui glitches.  The patch is largely the same, just one chunk removed from the beta version of the patch.

Comment 10

[Shane Caraveo (:mixedpuppy)]

Attachment: fix button states - beta

[Approval Request Comment]
Bug caused by (feature/regressing bug #): australis changes
User impact if declined: visual impact in button appearance in share and marks
Testing completed (on m-c, etc.): m-c, manually on beta
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fa072a5c34de

Comment 12

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/releases/mozilla-beta/rev/33d1476deae6