Last Comment Bug 772228 - Search Box in Tabview (Group Your Tabs) Does Not Resize When Out of View
: Search Box in Tabview (Group Your Tabs) Does Not Resize When Out of View
Status: RESOLVED FIXED
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: -- minor
: Firefox 17
Assigned To: Miles Sandlar
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-09 14:35 PDT by Miles Sandlar
Modified: 2016-04-12 14:00 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fixes Search Window in Panorama when the Window is Resized to be too Small (915 bytes, patch)
2012-07-10 10:12 PDT, Miles Sandlar
no flags Details | Diff | Review
Fixes the Search Box when Resizing in Panorama (Win/Mac/Linux, LTR/RTL) (2.02 KB, patch)
2012-07-17 15:55 PDT, Miles Sandlar
ttaubert: feedback+
Details | Diff | Review
Fixes the Search Box when Resizing in Panorama (Win/Mac/Linux, LTR/RTL) (Spaces not Tabs) (2.02 KB, patch)
2012-07-19 14:27 PDT, Miles Sandlar
ttaubert: review+
Details | Diff | Review
Fixes the Search Box when Resizing in Panorama (Win/Mac/Linux, LTR/RTL) (Spaces not Tabs) (1.63 KB, patch)
2012-07-23 09:39 PDT, Miles Sandlar
miles: checkin+
Details | Diff | Review

Description Miles Sandlar 2012-07-09 14:35:31 PDT
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.4+ (KHTML, like Gecko) Version/5.0 Safari/535.4+ dwb-hg/rev. 1025

Steps to reproduce:

Entered the Tabview (Group Your Tabs Button or Control+Shift+E).
Clicked on the Search Button (Magnifier Glass Icon).
Typed some text.
Resized my Firefox window to be narrower (width-wise) than the tab search box.


Actual results:

The Search Box did not resize and the text I typed was cut off on the left side.


Expected results:

The Search Box should have resized accordingly as the window became smaller than the Search Box.

I believe this is a matter of setting max-width to the Search Box rather than width. I'm planning on submitting a patch for this.
Comment 1 Miles Sandlar 2012-07-10 10:12:27 PDT
Created attachment 640657 [details] [diff] [review]
Fixes Search Window in Panorama when the Window is Resized to be too Small

This patch should work for Linux/GTK.
I think these changes would also work if applied to browser/components/tabview/tabview.css . I'm not sure what Mozilla best practices are or whether this should go in the tabview component (browser/components/tabview/tabview.css) or each theme (browser/themes/[theme]/tabview/tabview.css) as there seems to be a fair amount of overlap.

Any guidance would be greatly appreciated.
Comment 2 Virgil Dicu [:virgil] [QA] 2012-07-11 00:05:53 PDT
https://developer.mozilla.org/En/Developer_Guide/How_to_Submit_a_Patch
You should talk to Panorama's module owner about this patch: if he feels it's acceptable at this point.
You can also ask for guidance from developers on IRC (#developers or #introduction channel)

Tim, tentatively adding you here. Is this something which you would want added?
Comment 3 Tim Taubert [:ttaubert] 2012-07-16 09:05:46 PDT
Comment on attachment 640657 [details] [diff] [review]
Fixes Search Window in Panorama when the Window is Resized to be too Small

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

Hey Miles, thank you for taking a stab at this!

First, you only changed the Linux theme. We have one tabview.css that is common to all platforms, and three files that are specific to either Windows, Linux or Mac. Second, your changes will also need to work in RTL mode, which would affect "margin-left", "float: right", etc.


We can have all this much easier and should to the following in the tabview.css file for each theme:

1) Let's keep the 'width: 270px' for #searchbox but add a 'max-width: -moz-available'. So we're 270px wide unless there is less space available.

2) To keep a left border of 20px, we can now also apply a left margin to #searchbox. We shouldn't use margin-left, because that wouldn't make any sense in for RTL locales. We can use '-moz-margin-start: 20px' for that. This basically translates to 'margin-left' in LTR and to 'margin-right' in RTL mode.


If you have a new patch ready please ask me for review again and/or ping me on IRC if you need help!
Comment 4 Miles Sandlar 2012-07-17 15:55:21 PDT
Created attachment 643169 [details] [diff] [review]
Fixes the Search Box when Resizing in Panorama (Win/Mac/Linux, LTR/RTL)

Thanks for the reply Tim.

Let me know if this new patch is good or there are any more problems.
Comment 5 Tim Taubert [:ttaubert] 2012-07-18 02:38:33 PDT
Comment on attachment 643169 [details] [diff] [review]
Fixes the Search Box when Resizing in Panorama (Win/Mac/Linux, LTR/RTL)

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

Thank you, Miles! This looks great but you seem to use tabs for indentation, we normally use spaces. I'd really appreciate if you could fix this before we can land it!
Comment 6 Tim Taubert [:ttaubert] 2012-07-18 02:39:26 PDT
BTW, asking me for review was the right thing to do. Not sure why you removed the flag afterwards?
Comment 7 Miles Sandlar 2012-07-19 14:27:04 PDT
Created attachment 644016 [details] [diff] [review]
Fixes the Search Box when Resizing in Panorama (Win/Mac/Linux, LTR/RTL) (Spaces not Tabs)

Hi Tim,

Here's the revised patch using spaces instead of tabs. I unchecked the review last time because I wasn't sure if it was the right action. Thanks for clarifying.

Let me know if anything else is needed for this to get upstreamed.

Miles
Comment 8 Tim Taubert [:ttaubert] 2012-07-23 06:05:31 PDT
Comment on attachment 644016 [details] [diff] [review]
Fixes the Search Box when Resizing in Panorama (Win/Mac/Linux, LTR/RTL) (Spaces not Tabs)

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

Thank you, Miles! I'll push the patch in a few minutes and it should be merged to trunk soon.
Comment 9 Tim Taubert [:ttaubert] 2012-07-23 06:09:27 PDT
Oops, I was too hasty. We first need to add some meta information to the patch so that we can check it in. Here's a good description of how to do that:

http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

After uploading the patch that is now ready for checkin, you can add the keyword "checkin-needed" to this bug and someone will push it for you.

Thanks!
Comment 10 Miles Sandlar 2012-07-23 09:39:03 PDT
Created attachment 644959 [details] [diff] [review]
Fixes the Search Box when Resizing in Panorama (Win/Mac/Linux, LTR/RTL) (Spaces not Tabs)

Tim - thanks. This patch should now be ready for checkin.
Comment 11 Tim Taubert [:ttaubert] 2012-07-23 09:48:36 PDT
https://hg.mozilla.org/integration/fx-team/rev/20c801d85f36

Pushed to fx-team. Thank you, Miles! Your fix should be included in tomorrow's Nightly.

Small hint for next time: we mark the patch itself as checkin+ after we pushed it. The bug has a "keywords" field at the top. This is where you need to select checkin-needed to let someone else know that you have a patch that needs to land. Anyway, doesn't matter for this bug anymore :)
Comment 12 Tim Taubert [:ttaubert] 2012-07-24 09:26:45 PDT
https://hg.mozilla.org/mozilla-central/rev/20c801d85f36

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