Last Comment Bug 660229 - about:permissions left-side list auto resizes when string not found
: about:permissions left-side list auto resizes when string not found
Status: VERIFIED FIXED
:
Product: Firefox
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 7
Assigned To: :Margaret Leibovic
:
Mentors:
Depends on:
Blocks: 573176
  Show dependency treegraph
 
Reported: 2011-05-27 07:52 PDT by George Carstoiu
Modified: 2011-06-03 01:27 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.11 KB, patch)
2011-05-30 09:04 PDT, :Margaret Leibovic
no flags Details | Diff | Splinter Review
patch v2 (3.64 KB, patch)
2011-05-30 13:33 PDT, :Margaret Leibovic
dao+bmo: review-
Details | Diff | Splinter Review
patch v3 (4.32 KB, patch)
2011-05-31 03:27 PDT, :Margaret Leibovic
dao+bmo: review-
Details | Diff | Splinter Review
patch v4 (4.31 KB, patch)
2011-05-31 04:14 PDT, :Margaret Leibovic
dao+bmo: review+
Details | Diff | Splinter Review

Description George Carstoiu 2011-05-27 07:52:23 PDT
Mozilla/5.0 (Windows NT 5.1; rv:6.0a2) Gecko/20110526 Firefox/6.0a2

When entering an invalid search string in the left side list of about:permissions, the pane auto-resizes. 

Reproducible:always

Steps to reproduce:
1. Go to about:permissions
2. Type in an invalid search string

Actual results:
 - the pane auto-resizes 

Expected results:
 - nothing should happen
Comment 1 Thomas Ahlblom 2011-05-28 05:04:18 PDT
Reproduced.
Mozilla/5.0 (X11; Linux x86_64; rv:7.0a1) Gecko/20110527 Firefox/7.0a1
Comment 2 :Margaret Leibovic 2011-05-30 09:04:17 PDT
Created attachment 536109 [details] [diff] [review]
patch

This is happening because the width was set on the site richlistitem elements, instead of the richlistbox, so when no sites are visible, the width style doesn't do anything.

I only tested this patch on OSX right now, but it works as expected. Also, it doesn't affect the way really long domain names are handled.
Comment 3 :Margaret Leibovic 2011-05-30 13:33:10 PDT
Created attachment 536158 [details] [diff] [review]
patch v2

This patch is a more robust solution that fixes potential problems we might have with different font sizes by using em for the label width instead of px.
Comment 4 :Margaret Leibovic 2011-05-30 14:36:25 PDT
Comment on attachment 536158 [details] [diff] [review]
patch v2

Switching the review request to Dão, since he's more familiar with these theme issues.
Comment 5 Dão Gottwald [:dao] 2011-05-30 15:21:36 PDT
Comment on attachment 536158 [details] [diff] [review]
patch v2

>+#sites-list {
>+  width: -moz-calc(28px + 15em); /* .site padding, .site-favicon width and padding, .site-domain max-width */
>+}
>+
> .site {
>-  width: 250px;
>   overflow-x: hidden;
>   padding: 4px;
>   border-bottom: 1px solid ThreeDLightShadow;
> }

> .site-domain {
>-  max-width: 200px; /* crop set in XBL will ellipsize the domain if it's too long */
>+  max-width: 15em; /* crop set in XBL will ellipsize the domain if it's too long */
> }

.site-domain should flex rather than having its own max-width.

Why does .site have overflow-x: hidden, by the way?
Comment 6 :Margaret Leibovic 2011-05-30 15:38:40 PDT
(In reply to comment #5)
> .site-domain should flex rather than having its own max-width.

I wanted to set the max-width on .site-domain so that the crop attribute would take care of ellipsizing the text for me. I tried a few different approaches, but this seemed to be the one that works.

> Why does .site have overflow-x: hidden, by the way?

Without that style, if there is a long domain name that gets cropped the .site element will still be as wide as the uncropped label, creating an unwanted horizontal scrollbar on the richlistbox. Setting overflow-x: hidden; on #sites-list does not fix this problem.
Comment 7 Dão Gottwald [:dao] 2011-05-30 22:24:48 PDT
.site-domain and its containers need flex="1".
Comment 8 :Margaret Leibovic 2011-05-31 03:27:49 PDT
Created attachment 536265 [details] [diff] [review]
patch v3

Ah, this works! Dão, you are way more of a xul ninja than me :)
Comment 9 Dão Gottwald [:dao] 2011-05-31 03:53:44 PDT
Comment on attachment 536265 [details] [diff] [review]
patch v3

I think you should still use em rather than px. Or maybe even better yet, size sites-box and permissions-box relatively to each other by using different flex values.
Comment 10 :Margaret Leibovic 2011-05-31 04:14:42 PDT
Created attachment 536269 [details] [diff] [review]
patch v4

I prefer the idea of just using a fixed em width instead of flex because I'm worried some users might end up with awkwardly wide site lists if they are using a maximized window or a high resolution screen. There's no reason the site list needs to be much wider than the majority of domain names.
Comment 11 :Margaret Leibovic 2011-05-31 14:46:22 PDT
http://hg.mozilla.org/mozilla-central/rev/d2586300a6a7
Comment 12 Simona B [:simonab] 2011-06-03 01:27:46 PDT
Mozilla/5.0 (Windows NT 5.1; rv:7.0a1) Gecko/20110602 Firefox/7.0a1

Verified issue using the STR from Comment 1 on Win XP, Win 7, Mac X 10.6 and Ubuntu.

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