1032932 - 2.78% Win7 TResize regression from fx-team revision 920154597e3b on June 27th

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

This is a small tresize regression that you can see on the graph server:
http://graphs.mozilla.org/graph.html#tests=[[254,131,25],[254,132,25],[254,94,25]]&sel=1403622101603,1404226901603&displayrange=7&datatype=running

I did some retriggers:
https://tbpl.mozilla.org/?tree=Fx-Team&jobname=Windows%207%2032-bit%20fx-team%20talos%20chromez&fromchange=38b404a28e4e&tochange=570982344606

It looks like this revision:
https://hg.mozilla.org/integration/fx-team/rev/920154597e3b

caused us to go from ~22.1 -> ~22.7

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Shane, can you take a look at this.  Luckily this is only Win7 and a small regression, but if there is something simple we can do lets try it out.

Comment 2

[:Gijs (he/him)]

    2.10          <toolbarbutton id="social-share-button"
    2.11                         class="toolbarbutton-1 chromeclass-toolbar-additional"
    2.12 -                       hidden="true"

Ugh. I should have caught this in review. I thought this node was in the palette. It being on the toolbar means we shouldn't unhide it by default, I don't think. :-\

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: fix talos regression with share button

This is a try with some small changes so the button can remain hidden on startup.  Lets see if that addresses the regression.

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

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

The patch that caused this just got pushed to Aurora, so placing the necessary tracking flags on this so we make sure to uplift this patch to Aurora32 as well.

Comment 5

[:Gijs (he/him)]

Comment on attachment 8449531 [details] [diff] [review]
fix talos regression with share button

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

::: browser/base/content/browser-social.js
@@ +530,5 @@
>  
>    get shareButton() {
>      // web-panels (bookmark/sidebar) don't include customizableui, so
>      // nsContextMenu fails when accessing shareButton, breaking
>      // browser_bug409481.js.

Doesn't this mean you need to fix up nsContextMenu as well? And, from looking at mxr, MozSocialAPI ?

For better or for worse, this is a public API, and changing its return type might have unintended consequences. :-\

A safer patch (for uplift as well) would just change the update method to use CUI.

Comment 6

[Shane Caraveo (:mixedpuppy)]

Attachment: fix talos regression with share button

simpler approach from Gijs.

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

Comment 7

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/integration/fx-team/rev/24b11947a244

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Thanks guys for getting this fixed!

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/24b11947a244

Comment 10

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8449607 [details] [diff] [review]
fix talos regression with share button

Approval Request Comment
[Feature/regressing bug #]: 1014254
[User impact if declined]: talos regression due to share button being unecessarily visible
[Describe test coverage new/current, TBPL]: current
[Risks and why]: low
[String/UUID change made/needed]: none

Comment 11

[Shane Caraveo (:mixedpuppy)]

another note: bug 1014254 was uplifted to aurora, but this fix has not

Comment 12

[cmtalbert]

Not only is there a talos regression if we don't uplift to aurora, it also causes a confusing UX. The share button shows up when you have no providers installed and it is perma-disabled. So, that's kind of weird. I would add that as another reason we should uplift this.

Comment 13

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8449607 [details] [diff] [review]
fix talos regression with share button

Aurora+

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8697de0fdca3

Comment 15

[Ryan VanderMeulen [:RyanVM]]

I accidentally backed this out when I meant to backout a different bug for bustage. Re-landed.
https://hg.mozilla.org/releases/mozilla-aurora/rev/f4397dde382d