Last Comment Bug 694084 - Showing/hiding the conditional forward button resizes the search bar in a new profile (when the url/search bar splitter hasn't been used)
: Showing/hiding the conditional forward button resizes the search bar in a new...
Status: RESOLVED FIXED
[qa+][qa!:11][testday-20120203]
:
Product: Firefox
Classification: Client Software
Component: Toolbars and Customization (show other bugs)
: Trunk
: All All
-- normal with 1 vote (vote)
: Firefox 12
Assigned To: Dão Gottwald [:dao]
:
: :Gijs
Mentors:
Depends on:
Blocks: 674744 677027 682534
  Show dependency treegraph
 
Reported: 2011-10-12 11:33 PDT by Dão Gottwald [:dao]
Modified: 2012-02-10 06:57 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
+
verified


Attachments
patch (2.33 KB, patch)
2011-10-17 08:07 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review
patch v2 (2.72 KB, patch)
2011-11-02 08:14 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review
patch v3 (2.79 KB, patch)
2011-12-28 02:12 PST, Dão Gottwald [:dao]
gavin.sharp: review+
akeybl: approval‑mozilla‑aurora+
bugzilla: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description User image Dão Gottwald [:dao] 2011-10-12 11:33:11 PDT

    
Comment 1 User image Dão Gottwald [:dao] 2011-10-17 08:07:48 PDT
Created attachment 567449 [details] [diff] [review]
patch

Setting the width attribute by default along with flex makes layout behave consistently for profiles where the splitter has been touched and profiles where it hasn't been touched.
Comment 2 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-19 13:09:21 PDT
Comment on attachment 567449 [details] [diff] [review]
patch

This seems to make the search bar much narrower by default in new profiles:

Without patch: http://cl.ly/3X3y0W2Z0u1h1r3y0s3b
With patch: http://cl.ly/0m1L0211371S1q3J1K1A
Comment 3 User image Dão Gottwald [:dao] 2011-10-19 13:14:21 PDT
Hm, I did the same thing (comparing two screenshots) and there was no difference...
Comment 4 User image Dão Gottwald [:dao] 2011-11-02 08:14:12 PDT
Created attachment 571334 [details] [diff] [review]
patch v2
Comment 5 User image Alex Keybl [:akeybl] 2011-11-28 13:35:31 PST
[Triage Comment]
Once this is reviewed and landed on m-c, please nominate for aurora approval.
Comment 6 User image Dão Gottwald [:dao] 2011-12-28 02:12:32 PST
Created attachment 584540 [details] [diff] [review]
patch v3

updated to avoid the setTimeout
Comment 7 User image Jared Wein [:jaws] (please needinfo? me) 2012-01-02 23:56:28 PST
Comment on attachment 584540 [details] [diff] [review]
patch v3

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

::: browser/base/content/browser.js
@@ +2607,5 @@
>      splitter.parentNode.removeChild(splitter);
>  }
>  
> +function setUrlAndSearchBarWidthForConditionalForwardButton() {
> +  // Woraround for bug 694084: Showing/hiding the conditional forward button resizes

nit: s/Woraround/Workaround
Comment 8 User image Dietrich Ayala (:dietrich) 2012-01-12 08:44:11 PST
Comment on attachment 584540 [details] [diff] [review]
patch v3

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

maybe i'm just not familiar enough with the code here, but this seems like something that should be fixable with css alone. why all this code around a sizing issue?
Comment 9 User image Dão Gottwald [:dao] 2012-01-12 08:53:05 PST
This could be fixed in CSS if percenrage widths worked with the xul flexbox layout. Unfortunatrly, they don't.
Comment 11 User image Marco Bonardo [::mak] 2012-01-13 03:00:52 PST
https://hg.mozilla.org/mozilla-central/rev/c3956cae4197
Comment 12 User image Dão Gottwald [:dao] 2012-01-13 08:20:26 PST
(In reply to Jared Wein [:jwein and :jaws] from comment #7)
> nit: s/Woraround/Workaround

http://hg.mozilla.org/integration/mozilla-inbound/rev/8a0d9d68c5b0
Comment 13 User image Marco Bonardo [::mak] 2012-01-14 01:43:06 PST
https://hg.mozilla.org/mozilla-central/rev/8a0d9d68c5b0
Comment 14 User image Dão Gottwald [:dao] 2012-01-16 15:23:36 PST
Comment on attachment 584540 [details] [diff] [review]
patch v3

[Approval Request Comment]
Regression caused by (bug #): bug 674744, bug 677027, bug 682534 
User impact if declined: distracting movement when clicking the back button
Testing completed (on m-c, etc.): baking on mozilla-central
Risk to taking this patch (and alternatives if risky): no risk anticipated
Comment 15 User image Alex Keybl [:akeybl] 2012-01-17 09:47:42 PST
Comment on attachment 584540 [details] [diff] [review]
patch v3

[Triage Comment]
The user impact does not appear significant enough to meet the criteria for beta approval. That being said, we can expedite pushing this to our GA users by landing on Aurora.
Comment 16 User image Dão Gottwald [:dao] 2012-01-17 15:39:42 PST
Comment on attachment 584540 [details] [diff] [review]
patch v3

Requesting approval again. This is a regression in one of the most used parts of the primary UI. We also know this can be annoying -- I don't know about feedback from beta users, but nightly users asked how to disable the feature because of this.
Comment 18 User image Alex Keybl [:akeybl] 2012-01-19 12:13:53 PST
(In reply to Dão Gottwald [:dao] from comment #16)
> Comment on attachment 584540 [details] [diff] [review]
> patch v3
> 
> Requesting approval again. This is a regression in one of the most used
> parts of the primary UI. We also know this can be annoying -- I don't know
> about feedback from beta users, but nightly users asked how to disable the
> feature because of this.

We'll discuss this at today's channel meeting. It's difficult to make the case for taking this on our second-to-last or last beta given how long we've known about this issue. If it were that critical to our users, I would have expected us to land a patch much sooner.
Comment 19 User image Marcia Knous [:marcia - use ni] 2012-01-19 14:58:14 PST
Adding qawanted since it was noted in the channel meeting that it has landed on aurora and trunk.
Comment 20 User image Dão Gottwald [:dao] 2012-01-19 15:00:21 PST
(In reply to Marcia Knous [:marcia] from comment #19)
> Adding qawanted since it was noted in the channel meeting that it has landed
> on aurora and trunk.

Did you mean to add the keyword instead of whiteboard tag? Also, verifyme instead of qawanted?
Comment 21 User image Johnathan Nightingale [:johnath] 2012-01-19 15:01:35 PST
Comment on attachment 584540 [details] [diff] [review]
patch v3

Discussed in triage - approved, but we worry about taking changes to primary UI in a late beta. Please land it quickly, and think a little harder about cases where this could break, so that QA can test. (Very small window sizes? Using the resizer afterwards? Extensions that care about where they live in the toolbar?)
Comment 23 User image [:Aleksej] 2012-02-03 08:03:37 PST
On Linux-x86_64:

bug: Nightly 20111118 Firefox/11.0a1
WFM: 20120202 Firefox/13.0a1 and 11.0b1

bug: Aurora 20111118 Firefox/10.0a2
WFM: 20120202 Firefox/12.0a2
Comment 24 User image Remus Pop (:RemusPop) 2012-02-10 06:47:39 PST
Tested in Firefox 11 beta 2 on Ubuntu 11.04, Win XP and OSX 10.6.8:
Build identifier: Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0
Build identifier: Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko/20100101 Firefox/11.0
Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0

The search box does not change it's dimensions when the forward button is overlapped or not by the location bar.

Note You need to log in before you can comment on or make changes to this bug.