about:permissions left-side list auto resizes when string not found

VERIFIED FIXED in Firefox 7

Status

()

defect
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: george.carstoiu, Assigned: Margaret)

Tracking

Trunk
Firefox 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

8 years ago
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

8 years ago
Reproduced.
Mozilla/5.0 (X11; Linux x86_64; rv:7.0a1) Gecko/20110527 Firefox/7.0a1
Hardware: x86 → All
(Assignee)

Comment 2

8 years ago
Posted patch patch (obsolete) — Splinter Review
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.
Assignee: nobody → margaret.leibovic
Status: NEW → ASSIGNED
Attachment #536109 - Flags: review?(gavin.sharp)
(Assignee)

Updated

8 years ago
Blocks: 573176
(Assignee)

Comment 3

8 years ago
Posted patch patch v2 (obsolete) — Splinter Review
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.
Attachment #536109 - Attachment is obsolete: true
Attachment #536109 - Flags: review?(gavin.sharp)
Attachment #536158 - Flags: review?(gavin.sharp)
(Assignee)

Comment 4

8 years ago
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.
Attachment #536158 - Flags: review?(gavin.sharp) → review?(dao)
(Assignee)

Updated

8 years ago
Component: Preferences → Theme
QA Contact: preferences → theme
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?
Attachment #536158 - Flags: review?(dao) → review-
(Assignee)

Comment 6

8 years ago
(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.
.site-domain and its containers need flex="1".
(Assignee)

Comment 8

8 years ago
Posted patch patch v3 (obsolete) — Splinter Review
Ah, this works! Dão, you are way more of a xul ninja than me :)
Attachment #536158 - Attachment is obsolete: true
Attachment #536265 - Flags: review?(dao)
(Assignee)

Updated

8 years ago
Attachment #536265 - Attachment description: patch → patch v3
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.
Attachment #536265 - Flags: review?(dao) → review-
(Assignee)

Comment 10

8 years ago
Posted patch patch v4Splinter Review
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.
Attachment #536265 - Attachment is obsolete: true
Attachment #536269 - Flags: review?(dao)
Attachment #536269 - Flags: review?(dao) → review+
(Assignee)

Comment 11

8 years ago
http://hg.mozilla.org/mozilla-central/rev/d2586300a6a7
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 7
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.