SocialMarks updating fails when located in menu panel

VERIFIED FIXED in Firefox 32

Status

()

Firefox
SocialAPI
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Tracking

({regression})

32 Branch
Firefox 34
x86
Mac OS X
regression
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox32+ verified, firefox33+ verified, firefox34+ verified)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

3 years ago
Created attachment 8476366 [details] [diff] [review]
beta 32 patch

This is a regression from bug 1003523

- button is not updated when located in menu panel
- panel does not size correctly (only affects 32 since this code has changed in 33) after moving from menu-panel to toolbar

STR for button state updating:

- activate pocket or demo provider
- move button into menu panel
- open menu panel

EXPECTED:
- button is enabled
GOT:
- button is disabled

- open new blank tab
- open menu panel
- button is disabled
- switch to previous loaded tab
- open menu panel

EXPECTED:
- button is enabled
GOT:
- button is disabled
Attachment #8476366 - Flags: review?(jaws)
(Assignee)

Updated

3 years ago
status-firefox32: --- → affected
status-firefox33: --- → affected
status-firefox34: --- → affected
(Assignee)

Comment 1

3 years ago
Fixing this bug is slightly more complex on 33/34 due to bug 1011598 (landed on 33) which removed using panelviews for the socialmark button, decided to get the beta patch up first.
(Assignee)

Comment 2

3 years ago
Created attachment 8476376 [details] [diff] [review]
beta 32 patch

sigh...get the right patch
Assignee: nobody → mixedpuppy
Attachment #8476366 - Attachment is obsolete: true
Attachment #8476366 - Flags: review?(jaws)
Attachment #8476376 - Flags: review?(jaws)
(Assignee)

Comment 3

3 years ago
moved sizing bug to bug 1056422, wontfix for 32
(Assignee)

Comment 4

3 years ago
Created attachment 8476414 [details] [diff] [review]
fix update for buttons in menu-panel (nightly)

patch for nightly fx34
Attachment #8476414 - Flags: review?(jaws)
(Assignee)

Comment 5

3 years ago
Created attachment 8476416 [details] [diff] [review]
fix update for buttons in menu-panel (aurora)

same as nightly patch, made to apply on aurora
Attachment #8476416 - Flags: review?(jaws)
(Assignee)

Comment 6

3 years ago
https://tbpl.mozilla.org/?tree=Try&rev=aabd85ca6600
Comment on attachment 8476414 [details] [diff] [review]
fix update for buttons in menu-panel (nightly)

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

::: browser/base/content/browser-social.js
@@ +302,5 @@
>        return false;
>      return Social.providers.length > 0;
>    },
>  
> +  updatePanelState :function(event) {

nit, updatePanelState: function(event) {

@@ +1389,1 @@
>      // querySelectorAll does not work on the menu panel the panel, so we have to

might as well fix this comment to make more sense, "the menu panel the panel" ;)
Attachment #8476414 - Flags: review?(jaws) → review+
Comment on attachment 8476376 [details] [diff] [review]
beta 32 patch

r=me except I'm curious about the following change:

>+          this._useDynamicResizer = !size;
>           let {width, height} = size ? size : {width: 330, height: 100};
>-          this._useDynamicResizer = !size;

Can you explain why moving this is necessary?
Attachment #8476376 - Flags: review?(jaws) → review+
Comment on attachment 8476416 [details] [diff] [review]
fix update for buttons in menu-panel (aurora)

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

::: browser/base/content/browser-social.js
@@ +286,5 @@
>        return false;
>      return Social.providers.length > 0;
>    },
>  
> +  updatePanelState :function(event) {

nit, updatePanelState: function(event) {
Attachment #8476416 - Flags: review?(jaws) → review+
(Assignee)

Comment 10

3 years ago
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)
> Comment on attachment 8476376 [details] [diff] [review]
> beta 32 patch
> 
> r=me except I'm curious about the following change:
> 
> >+          this._useDynamicResizer = !size;
> >           let {width, height} = size ? size : {width: 330, height: 100};
> >-          this._useDynamicResizer = !size;
> 
> Can you explain why moving this is necessary?

It's not and is removed from my local patch, noticed it after uploading, will update after the tests complete and I can land it.
(Assignee)

Comment 11

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/cfff821ca490
(Assignee)

Comment 12

3 years ago
Comment on attachment 8476414 [details] [diff] [review]
fix update for buttons in menu-panel (nightly)

Approval Request Comment
[Feature/regressing bug #]: 1003523
[User impact if declined]: marks fails to work if customized into menu panel
[Describe test coverage new/current, TBPL]: current coverage is only when button is in toolbar, manual testing for menu panel
[Risks and why]: low
[String/UUID change made/needed]: none
Attachment #8476414 - Flags: approval-mozilla-beta?
Attachment #8476414 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 13

3 years ago
Created attachment 8476789 [details] [diff] [review]
fix update for buttons in menu-panel (beta)

patch update on earlier comment
Attachment #8476789 - Flags: review+
tracking-firefox32: --- → +
tracking-firefox33: --- → +
tracking-firefox34: --- → +
Keywords: regression
Shane, given this is a regression, does this have tests or do we need to add some?
(Assignee)

Comment 15

3 years ago
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #14)
> Shane, given this is a regression, does this have tests or do we need to add
> some?

we'll need to add some
(In reply to Shane Caraveo (:mixedpuppy) from comment #15)
> (In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #14)
> > Shane, given this is a regression, does this have tests or do we need to add
> > some?
> 
> we'll need to add some

Please let me know if you need QA's help with that (manual or automated).
(Assignee)

Updated

3 years ago
Attachment #8476376 - Attachment is obsolete: true
Florin, this should be landing across branches and uplifting to Beta tonight for 32.0b9. Please be sure to verify this as part of your sign offs.
Flags: qe-verify+
Flags: needinfo?(florin.mezei)
https://hg.mozilla.org/mozilla-central/rev/cfff821ca490
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox34: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Comment on attachment 8476789 [details] [diff] [review]
fix update for buttons in menu-panel (beta)

Now that Shane has verified this fix on fx-team, beta+
Attachment #8476789 - Flags: approval-mozilla-beta+
Attachment #8476416 - Flags: approval-mozilla-aurora+
Comment on attachment 8476414 [details] [diff] [review]
fix update for buttons in menu-panel (nightly)

Clearing approval requests as I've approved the aurora and beta specific patches.
Attachment #8476414 - Flags: approval-mozilla-beta?
Attachment #8476414 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/releases/mozilla-aurora/rev/e5fdb23a3914
https://hg.mozilla.org/releases/mozilla-beta/rev/2f61f6e44a33
status-firefox32: affected → fixed
status-firefox33: affected → fixed
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:32.0) Gecko/20100101 Firefox/32.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:33.0) Gecko/20100101 Firefox/33.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:34.0) Gecko/20100101 Firefox/34.0

Verified fixed on Mac OS X 10.9.4 using the STR from the description on:
- Firefox 32 beta 9, build ID: 20140822024446
- latest Aurora, build ID: 20140822004002
- latest Nightly, build ID: 20140822030201
Status: RESOLVED → VERIFIED
status-firefox32: fixed → verified
status-firefox33: fixed → verified
status-firefox34: fixed → verified
Flags: needinfo?(florin.mezei)
You need to log in before you can comment on or make changes to this bug.