Closed Bug 477948 Opened 11 years ago Closed 6 years ago

"Keyhole" back/forward present on every major platform except Linux

Categories

(Firefox :: Theme, defect, minor)

All
Linux
defect
Not set
minor

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox29 --- verified
firefox30 --- verified
firefox31 --- verified

People

(Reporter: from_bugzilla2, Assigned: mikedeboer)

References

Details

(Whiteboard: [Australis:P?])

Attachments

(3 files, 9 obsolete files)

109.68 KB, image/png
Details
3.31 KB, patch
jaws
: review+
Details | Diff | Splinter Review
16.16 KB, patch
mikedeboer
: review+
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.6) Gecko/2009020905 Gentoo Firefox/3.0.6 Ubiquity/0.1.5
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.6) Gecko/2009020905 Gentoo Firefox/3.0.6 Ubiquity/0.1.5

The Keyhole-style back/forward/history construct is present in the toolbar on all major platforms except Linux.

While desktop integration is all well and good, identity should also be maintained and Firefox doesn't really feel like firefox with such a distinctive GUI element missing.

It doesn't help that addons.mozilla.org lies about the workaround (the "XP on Vista" and "Vista on XP" themes) being Linux-incompatible. (They work perfectly well)

Reproducible: Always
Component: Toolbars → Theme
QA Contact: toolbars → theme
I have been against the keyhole on linux for a long time now but seeing Steven's new (Fx4) mockups and hearing what people think about those vs native icons convinced me that at least having the choice to have the keyhole would be nice.

I would go one step further and make the keyhole default for all mozilla.org builds. Keeping GTK icon support should not be a big issue, so maybe distributions should decide if they ship a "mozilla identity" or "GTK integration" version of Firefox. Users could always switch to the other option.
Duplicate of this bug: 940322
Status: UNCONFIRMED → NEW
Ever confirmed: true
I'm assuming that as a result of the patch for this bug, the clipping on the download-finished icon will also be fixed? Or does that need a separate bug filed?
(In reply to Paul [sabret00the] from comment #4)
> I'm assuming that as a result of the patch for this bug, the clipping on the
> download-finished icon will also be fixed? Or does that need a separate bug
> filed?

This is already filed as bug 941737 :)
Attached patch WIP patch v2 (obsolete) — Splinter Review
Brandon, I've got this patch mostly done but the clip-path for the back button isn't exact, and I know that you're planning on making some changes to the back button size. Could you help out here with fixing the clip-path?
Attachment #8334677 - Attachment is obsolete: true
Flags: needinfo?(bcheng.gt)
Definitely. Things are slowing down for me in terms of school work for next week. I plan to be knocking out those two bugs I'm assigned to in that time.
Flags: needinfo?(bcheng.gt)
I assume you'd like the adjusted clip-path after bug 893661 is patched, right? In that case, we're waiting on the assets.
(In reply to Brandon Cheng from comment #8)
> I assume you'd like the adjusted clip-path after bug 893661 is patched,
> right? In that case, we're waiting on the assets.

Yes, that's correct. I'll see what I can do to get the assets sooner.
This patch also work in RTL mode and doesn't hijack the forward button's hover effect.
Assignee: nobody → mdeboer
Attachment #8361991 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8389213 - Flags: review?(jaws)
Comment on attachment 8389213 [details] [diff] [review]
Patch v1: Keyhole back/ forward button for Linux

We shouldn't hard-code non-native background and border colors for the url bar and search bar.
Attachment #8389213 - Flags: review-
(In reply to Dão Gottwald [:dao] from comment #11)
> We shouldn't hard-code non-native background and border colors for the url
> bar and search bar.

What about using -moz-field for the urlbar background and ThreeDShadow for the border?
Flags: needinfo?(dao)
(In reply to Mike de Boer [:mikedeboer] from comment #12)
> (In reply to Dão Gottwald [:dao] from comment #11)
> > We shouldn't hard-code non-native background and border colors for the url
> > bar and search bar.
> 
> What about using -moz-field for the urlbar background and ThreeDShadow for
> the border?

These are indeed the native colors that are supposed to be used for text fields. textbox.css uses them too.
Flags: needinfo?(dao)
Native colors win.
Attachment #8389213 - Attachment is obsolete: true
Attachment #8389213 - Flags: review?(jaws)
Attachment #8389266 - Flags: review?(jaws)
Comment on attachment 8389266 [details] [diff] [review]
Patch v1.1: Keyhole back/ forward button for Linux

What about the focused state? -moz-appearance: textfield automatically gave us a native glow or colored border and you don't seem to have implemented an replacement.
Comment on attachment 8389266 [details] [diff] [review]
Patch v1.1: Keyhole back/ forward button for Linux

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

::: browser/base/content/browser.xul
@@ +1168,1 @@
>      <svg:clipPath id="windows-keyhole-forward-clip-path" clipPathUnits="objectBoundingBox">

we should rename this mask so it is obvious that it is used for linux and windows.

::: browser/themes/linux/browser.css
@@ +14,5 @@
>  %filter substitution
>  
>  %define forwardTransitionLength 150ms
>  %define conditionalForwardWithUrlbar window:not([chromehidden~="toolbar"]) #urlbar-container
> +%define conditionalForwardWithUrlbarWidth 27 

nit, trailing whitespace
Attachment #8389838 - Flags: review?(jaws)
Attachment #8389266 - Attachment is obsolete: true
Attachment #8389266 - Flags: review?(jaws)
With the patch applied, the identity block is too close to the back button when the forward button is hidden. This doesn't look bad when there is a notification showing (like the Remember Password notification).
Comment on attachment 8389838 [details] [diff] [review]
Patch v1.2: Keyhole back/ forward button for Linux

r- for the issue in the previous comment. looks good otherwise.
Attachment #8389838 - Flags: review?(jaws) → review-
Attachment #8389838 - Attachment is obsolete: true
Attachment #8389920 - Attachment is obsolete: true
Attachment #8390169 - Flags: review?(jaws)
Attachment #8390169 - Flags: review?(jaws)
Now also mirroring the identity-box CSS from the Windows theme.
Attachment #8390169 - Attachment is obsolete: true
Attachment #8390192 - Flags: review?(jaws)
Comment on attachment 8390192 [details] [diff] [review]
Patch v1.4: Keyhole back/ forward button for Linux

I tested it on my Ubuntu VM and it now looks better. Ship it!
Attachment #8390192 - Flags: review?(jaws) → review+
Landed as https://hg.mozilla.org/integration/fx-team/rev/15516aebd58d
Whiteboard: [Australis:P?][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/15516aebd58d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P?][fixed-in-fx-team] → [Australis:P?]
Target Milestone: --- → Firefox 30
This caused two Talos regressions:

Regression: Fx-Team-Non-PGO - SVG, Opacity Row Major - Ubuntu HW 12.04 - 67.9% increase
---------------------------------------------------------------------------------------
   Previous: avg 157.792 stddev 36.136 of 12 runs up to revision 02be46ac2b55
   New     : avg 265.000 stddev 1.279 of 12 runs since revision 15516aebd58d
   Change  : +107.208 (67.9% / z=2.967)
   Graph   : http://mzl.la/1fC5A4G

and

Regression: Fx-Team-Non-PGO - Tab Animation Test - Ubuntu HW 12.04 - 5.75% increase
-----------------------------------------------------------------------------------
   Previous: avg 4.631 stddev 0.023 of 12 runs up to revision 02be46ac2b55
   New     : avg 4.897 stddev 0.029 of 12 runs since revision 15516aebd58d
   Change  : +0.266 (5.75% / z=11.482)
   Graph   : http://mzl.la/1fC5wSy

The SVG test regresses, because we're now using SVG clipping masks in the toolbar for the unified back/ forward button.

The TART test regresses, because of the same reason, actually.

I think both regression are expected & acceptable, thus I feel comfortable at this point to request uplift.
Comment on attachment 8390192 [details] [diff] [review]
Patch v1.4: Keyhole back/ forward button for Linux

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis (but really? WAY BACK! - bug 682534 and bug 674744)
User impact if declined: All platforms have the Firefox-trademark-keyhole, except the Linux flavors. It's time to not let them feel left out anymore. This patch does regress the 'TART' and 'SVG, Opacity Row Major' Talos tests with 5.75% and 67.9% respectively, but they are expected as explained above.
Testing completed (on m-c, etc.): landed on m-c.
Risk to taking this patch (and alternatives if risky): minor, no known regressions apart from Talos.
String or IDL/UUID changes made by this patch: n/a
Attachment #8390192 - Flags: approval-mozilla-aurora?
(In reply to Mike de Boer [:mikedeboer] from comment #25)
> The SVG test regresses, because we're now using SVG clipping masks in the
> toolbar for the unified back/ forward button.

Why does that regress an SVG test?

> The TART test regresses, because of the same reason, actually.

Why is it acceptable to regress tab animation performance here? How does TART performance on Linux compare to the other platforms, and does the keyhole have the same impact there?
Comment on attachment 8390192 [details] [diff] [review]
Patch v1.4: Keyhole back/ forward button for Linux

Doesn't seem like a viable uplift candidate until we sort out the performance regressions at the very least. I'm not convinced we should be spending time on this now.
Attachment #8390192 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Agreed, I won't be spending time on Talos regressions right now.
Should this be backed out?
Flags: needinfo?(gavin.sharp)
Sounds reasonable.
Flags: needinfo?(gavin.sharp)
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #27)
> (In reply to Mike de Boer [:mikedeboer] from comment #25)
> > The SVG test regresses, because we're now using SVG clipping masks in the
> > toolbar for the unified back/ forward button.
> 
> Why does that regress an SVG test?

It shouldn't, I wouldn't have thought. No idea why this happened, primarily because I thought this was a core/layout/gfx perf test, which shouldn't be affected by a change in the UI such as this one.

> > The TART test regresses, because of the same reason, actually.
> 
> Why is it acceptable to regress tab animation performance here? How does
> TART performance on Linux compare to the other platforms, and does the
> keyhole have the same impact there?

We don't know because there was no TART test at the time, because we didn't have TART, did we? It's a known issue with TART that it is affected by UI updates that happen when you switch tabs (id est updating the bookmarks star, back button state, identity indicator panel in the URL bar, etc. causes reflows, which make the animation frame rate drop, which regresses TART). I wouldn't have thought that this is any worse than the drag that's already being caused by the same stuff on Windows or Mac. In any case, that seems very hard to figure out.

As compared to other platforms, Linux is comfortably smoother (although I admittedly don't know how our talos reference platforms compare perf-wise and to what extent that skews the numbers):

http://graphs.mozilla.org/graph.html#tests=[[293,132,33],[293,132,37],[293,132,25]]&sel=1394699082554.1133,1394757890047.4229&displayrange=7&datatype=running
(In reply to :Gijs Kruitbosch from comment #32)
> We don't know because there was no TART test at the time, because we didn't
> have TART, did we? It's a known issue with TART that it is affected by UI
> updates that happen when you switch tabs (id est updating the bookmarks
> star, back button state, identity indicator panel in the URL bar, etc.
> causes reflows, which make the animation frame rate drop, which regresses
> TART). I wouldn't have thought that this is any worse than the drag that's
> already being caused by the same stuff on Windows or Mac. In any case, that
> seems very hard to figure out.

What about disabling "the same stuff" on Windows/Mac and pushing to talos?

In any case, leading-questions aside, my main point is just that regressing Talos tests requires more justification and thought than what was put into comment 25, particularly for changes of that magnitude, and especially if the win you're trading off isn't significant. We don't have a lot of time at the moment to do that investigation, given other Australis commitments, so let's back this out and revisit when we do.
Backed out on fx-team:
https://hg.mozilla.org/integration/fx-team/rev/46355c54bd1f
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 30 → ---
Depends on: 984317
Duplicate of this bug: 984714
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #27)
> Why does that regress an SVG test?

I added a note at https://wiki.mozilla.org/Buildbot/Talos/Tests#tsvg-opacity .

In a nutshell: all the page load tests measure from before the location is changed until onload. Therefore chrome perf from location change is measured as well. SVG opacity specifically is rather quick, so the changes in percentage is bigger than with other page load tests.

(In reply to Mike de Boer [:mikedeboer] from comment #25)
> This caused two Talos regressions:
> 
> Regression: Fx-Team-Non-PGO - SVG, Opacity Row Major - Ubuntu HW 12.04 - 67.9% increase
> ---------------------------------------------------------------------------------------
>    Previous: avg 157.792 stddev 36.136 of 12 runs up to revision 02be46ac2b55
>    New     : avg 265.000 stddev 1.279 of 12 runs since revision 15516aebd58d
>    Change  : +107.208 (67.9% / z=2.967)
>    Graph   : http://mzl.la/1fC5A4G

Specifically for this regression, if we ignore the percentage, we see 110ms regression, which, if indeed is a result of this patch, is probably from the chrome perf on location change, and IMO too much to land as is.
With Firefox about to undergo the massive promotion and scrutiny that comes with the launch of Australis we need to have a unified product. The keyhole is a part of the Firefox brand identity. And given that there's no movement on fixing regressions that bring a platform up to par with the rest, it's absurd that this was backed out in the first place. There's not even evidence to say that this is platform specific given the changes, just that Linux never got its keyhole before such effects were measurable. This is a case of branding versus performance, so I think it's a call for Product? But the argument for performance hasn't been made properly, thus at this point branding wins by default.
OK, let's split the discussion into the two regressions:

1. TART: this is "only" 6%, and at least from the graph linked from comment 32, Linux is better than other platforms to begin with, so unless someone can visually notice the regression from this patch with tab animation, I'd say that's not a blocker. But we do need someone with Linux to play for few mins with Firefox with and without the patch, and confirm that there's no noticeable regression in tab animation. Volunteers?

2. SVG Opacity. This is a tougher one. I'm looking at this graph: http://graphs.mozilla.org/graph.html#tests=[[225,132,33]]&sel=1387893593024,1395669593024&displayrange=90&datatype=running (m-i looks very similar).

Before Jan-16 it was around 130. Between Jan-16 and Feb-1 they were around 260. On Feb-1 they returned to ~130, and on Feb-19 they became multi-modal with buckets (in decreasing probability) at around 135, 175, 265 and few around 300 (with average around 160).

This patch apparently made the 260 bucket the dominant one, and it returned to the multi-modal behavior once it got backed out.

It's a hard call without understanding why the results became multi-modal to begin with and why did the patch make them more deterministic, however, considering the importance of keeping the brand identity, I'd say this also needs to be assessed by a human. I.e. load few pages with and without the patch and see if there's a noticeable difference.


We should remember that numbers are just numbers. The important factor is whether or not users will suffer from performance. So while the numbers help us notice things which we otherwise might not have, it's us which call the shots based on common sense, while taking the numbers into account as well.

So first, someone with native (not VM) linux should compare the behavior visually with and without the patch and report here. Then, assuming no major differences was observed wrt performance, I'd say this is not a blocker.

Regardless, since this patch affected 2 tests, it might be worth another review, maybe there are some low hanging fruits here.
Still at SVG Opacity, looking a year back, it seems that except for (not continuous) few months, this test has been multimodal for a long time. This increases the chance that this patch didn't necessarily regressed explicitly, but somehow made the test more deterministic on one of the (higher) buckets.

http://graphs.mozilla.org/graph.html#tests=[[225,132,33]]&sel=none&displayrange=365&datatype=running
I just tested those two builds on a Linux Mint machine we had here in the office. I didn't notice a discernible performance difference in tab animation, or general browser performance.
Thanks. OK, let's land this, while keeping an eye on performance other than SVG opacity.
I've compared both of these builds (well, actually, I tested the equivalent 64-bit builds) on my Ubuntu machine. I didn't notice any difference in performance between the 2 builds when opening / closing / switching between tabs and navigating within sites.
Something I wanted to see was how much difference the presence of an unused SVG clip path would make on the SVG Opacity talos test.

I pushed three csets to the try server so we can get data on this:
Baseline: https://tbpl.mozilla.org/?tree=Try&rev=ddedc49c62ad
With keyhole: https://tbpl.mozilla.org/?tree=Try&rev=0316e12a38c9
With keyhole + unused clip path: https://tbpl.mozilla.org/?tree=Try&rev=a1df4d0cfbb3

After I get the results I'm planning on re-landing the keyhole patch on fx-team.
The try results above show that adding an unused SVG clip path show a similar performance hit for SVG Opacity.

Landed on fx-team, https://hg.mozilla.org/integration/fx-team/rev/d8846585295f
Whiteboard: [Australis:P?] → [Australis:P?][fixed-in-fx-team]
Having performed the same test as Chris Coulson above, I came to the same conclusions on my 64-bit Ubuntu machine in the London office.
I tested on a i386 machine with ten tabs open and my results match those of Chris and :ato. It's fantastic news, as it's looking like we'll be able to launch complete Australis across all platforms at once.
I had to back this out in https://hg.mozilla.org/integration/fx-team/rev/d8846585295f for apparently breaking Windows XP debug mochitests.

Not sure how or why, really, but retriggers of tests point to this commit as the cause.

Failures look like this: https://tbpl.mozilla.org/php/getParsedLog.php?id=36633724&tree=Fx-Team
Flags: needinfo?(jaws)
Whiteboard: [Australis:P?][fixed-in-fx-team] → [Australis:P?]
Did the mochitests fail on the original landing on this patch too or is it just this time around?
(In reply to Paul [sabret00the] from comment #49)
> Did the mochitests fail on the original landing on this patch too or is it
> just this time around?

Just this time around. :/
(In reply to Paul [sabret00the] from comment #49)
> Did the mochitests fail on the original landing on this patch too or is it
> just this time around?

This time around. As discussed on IRC yesterday, this patch is affected by a recent change to the SVG paths (bug 893661), and therefore it now needs some more testing / updating. Please let Jared and Mike take care of it, they're more than capable. Thanks!
Mike, can you pick this back up and check that this displays correctly?
Flags: needinfo?(jaws) → needinfo?(mdeboer)
Absolutely!
Flags: needinfo?(mdeboer)
Status: REOPENED → ASSIGNED
Unbitrotted patch that works with the updated clip-paths. I don't have a working public key yet, so I can't push to try or anything. Jared, could you do the honors?
Attachment #8390192 - Attachment is obsolete: true
Attachment #8397119 - Flags: review?(jaws)
(In reply to Mike de Boer [:mikedeboer] from comment #54)
> Created attachment 8397119 [details] [diff] [review]
> Patch v1.5: Keyhole back/ forward button for Linux
> 
> Unbitrotted patch that works with the updated clip-paths.

I'm actually a bit curious to know. Does calculating an arc path instead of a cubic one perform better?
Comment on attachment 8397119 [details] [diff] [review]
Patch v1.5: Keyhole back/ forward button for Linux

This is breaking the combined bookmarks/bookmarks-menu button. See attached screenshot.
Attachment #8397119 - Flags: review?(jaws) → review-
Comment on attachment 8397119 [details] [diff] [review]
Patch v1.5: Keyhole back/ forward button for Linux

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

Setting margin-top: 3px; margin-bottom: 3px; on .toolbarbutton-menubutton-dropmarker fixes the issue for me. With that fixed you've got r+.
Attachment #8397119 - Flags: review- → review+
I had to update the notifications test, because test #15 places a normal button in the notification are, which has standard GTK styling and dimensions.
Due to the clip-path, this button gets clipped off, which makes it necessary to synthesize the mouse click a little bit downward. To be safe, I adjusted the x-offset with the same amount.
Attachment #8397849 - Flags: review?(jaws)
Margin fix. Carrying over r=jaws.
Attachment #8397119 - Attachment is obsolete: true
Attachment #8397928 - Flags: review+
Comment on attachment 8397849 [details] [diff] [review]
Patch 2: update test

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

::: browser/base/content/test/general/browser_popupNotification.js
@@ +180,5 @@
>  var tests = [
>    { // Test #0
>      run: function () {
>        this.notifyObj = new basicNotification();
> +      dump("0-1: " + this.notifyObj.id + "\n");

please remove these dumps

@@ +534,5 @@
>        this.notifyObj.anchorID = this.box.id = "nested-box";
>        this.notifyObj.addOptions({dismissed: true});
>        this.notification = showNotification(this.notifyObj);
>  
> +      EventUtils.synthesizeMouse(button, 4, 4, {});

Can you add a comment about needing to move the point due to the clip path?
Attachment #8397849 - Flags: review?(jaws) → review+
Pushed both patches with comments addressed:

https://hg.mozilla.org/integration/fx-team/rev/7baa06f4b49c
https://hg.mozilla.org/integration/fx-team/rev/7fe4e2d84d76

Thanks Jared!
Whiteboard: [Australis:P?] → [Australis:P?][fixed-in-fx-team]
I pushed to Try before, forgot to mention that here:

https://tbpl.mozilla.org/?tree=Try&rev=1973904f2e84
https://hg.mozilla.org/mozilla-central/rev/7baa06f4b49c
https://hg.mozilla.org/mozilla-central/rev/7fe4e2d84d76
Status: ASSIGNED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P?][fixed-in-fx-team] → [Australis:P?]
Target Milestone: --- → Firefox 31
Depends on: 989466
Depends on: 989701
Depends on: 990084
uplift nominations for aurora/beta?
Flags: needinfo?(mdeboer)
Comment on attachment 8397928 [details] [diff] [review]
Patch v1.6: Keyhole back/ forward button for Linux

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis
User impact if declined:

The 'keyhole' conditional back/ forward button is one of Firefox' most identifying UI elements, which is already present on Windows and OSX.
With Australis, the keyhole fits even better in the 'curvy' design.
This set of patches implements the keyhole on Linux, so these users also get this experience.

Testing completed (on m-c, etc.):

This has landed on m-c. Bug 989701 needs to be fixed first, before I push any patch to aurora or beta.

Risk to taking this patch (and alternatives if risky):

atm, no major regressions, apart from bug 989701, have been reported and the performance regressions have been analyzed and accepted. If anything blocking comes up on aurora or beta, the patches can be backed out quite easily (few to no dependencies).

String or IDL/UUID changes made by this patch: n/a
Attachment #8397928 - Flags: approval-mozilla-beta?
Attachment #8397928 - Flags: approval-mozilla-aurora?
Flags: needinfo?(mdeboer)
(In reply to Mike de Boer [:mikedeboer] from comment #67)
> performance regressions have been analyzed and accepted.

What about bug 990084?
Depends on: 990163
(In reply to Dão Gottwald [:dao] from comment #68)
> What about bug 990084?

Well, ehm, I didn't count that one yet... :/ So, I will hold off with uplifting any patches until the tresize regression is explained/ resolved.
Comment on attachment 8397928 [details] [diff] [review]
Patch v1.6: Keyhole back/ forward button for Linux

Refuse the uplift for now (cf comment #69). Don't hesitate to resubmit when the performance regression are fixed.
Attachment #8397928 - Flags: approval-mozilla-beta?
Attachment #8397928 - Flags: approval-mozilla-beta-
Attachment #8397928 - Flags: approval-mozilla-aurora?
Attachment #8397928 - Flags: approval-mozilla-aurora-
Pushed to Aurora to get a clearer picture of the regressions and I wanted to know if the tresize regression was truly caused by this bug.

baseline Try push: https://tbpl.mozilla.org/?tree=Try&rev=fcf193449da4
keyhole Try push: https://tbpl.mozilla.org/?tree=Try&rev=32c9583cea1c

http://compare-talos.mattn.ca/?oldRevs=fcf193449da4&newRev=32c9583cea1c&server=graphs.mozilla.org&submit=true

The table shows that the newly introduced keyhole does indeed regress all the things. We're OK with TART (bug 990163) and SVG Opacity regressing as they are (see comment 38), but tresize seems too much. We need more investigation here. This work will be tracked in bug 990084.
Hardware: x86 → All
Comment on attachment 8397928 [details] [diff] [review]
Patch v1.6: Keyhole back/ forward button for Linux

Please see the approval request in comment 67.
Attachment #8397928 - Flags: approval-mozilla-beta?
Attachment #8397928 - Flags: approval-mozilla-beta-
Attachment #8397928 - Flags: approval-mozilla-aurora?
Attachment #8397928 - Flags: approval-mozilla-aurora-
Attachment #8397928 - Flags: approval-mozilla-beta?
Attachment #8397928 - Flags: approval-mozilla-beta+
Attachment #8397928 - Flags: approval-mozilla-aurora?
Attachment #8397928 - Flags: approval-mozilla-aurora+
Mozilla/5.0 (X11; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (X11; Linux i686; rv:30.0) Gecko/20100101 Firefox/30.0
Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Firefox/31.0

Verified as fixed on Ubuntu 13.10 x86 using Firefox 29.0b8 and the 04/15 Firefox 30.0a2 and 31.0a1.
when this landed on beta, we see a 12-13% regression on windows 8 for the tart test, I verified by backing out the patch:
https://tbpl.mozilla.org/?tree=Try&rev=be431ebd29a0
Depends on: 998454
Depends on: 1004888
You need to log in before you can comment on or make changes to this bug.