Closed Bug 418879 Opened 12 years ago Closed 11 years ago

Long feed URLs cause scrollbar to appear in Page Info

Categories

(Firefox :: Page Info Window, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 3.1a1

People

(Reporter: elmar.ludwig, Assigned: ehsan)

References

()

Details

(Keywords: polish, regression)

Attachments

(5 files)

Attached image example screenshot
When the URL of a feed offered by a web site is fairly long, a horizontal scrollbar appears in the "Feeds" tab of the Page Info dialog. The user has to scroll all the way to the right to get to the "Subscribe" button, which is at least a bit awkward. This can be observed in many places in bugzilla, see the attached screenshot for one example.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b4pre) Gecko/2008022004 Minefield/3.0b4pre ID:2008022004
I think this is a regression from bug 383880.
Blocks: 383880
OS: Linux → All
Hardware: PC → All
Hmm, I tried adding crop="end" to the label containing the link, to no avail.  Is there a special XUL trick to use here?
Keywords: regression
Attached patch Patch (v1)Splinter Review
This patch solves the problem.  I added flex="1" and crop="end" to the label containing the link, and also increased the flex on the spacer next to it to 10000.  The reason is that without such huge flex, the spacer doesn't scale well, and some space right to the link's text would be clickable.
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
Attachment #305237 - Flags: review?(mano)
Keywords: polish
Target Milestone: --- → Firefox 3 beta4
(In reply to comment #2)
> Hmm, I tried adding crop="end" to the label containing the link, to no avail. 
> Is there a special XUL trick to use here?

FWIW, the trick here was to make the label flexible (e.g., by adding flex="1" to it).
(In reply to comment #5)
> Why is the spacer there anyway?

Without it, if there is a short feed URL which doesn't crop (for example, let's say its width is half of the list box width), and the user points the mouse cursor to the right of the link's text (where no text is shown) the cursor changes to the shape of the hand.  This is because the XUL label spans the whole width of the list box.  The spacer causes the label to be just as wide as it needs to be, therefore solving this problem.
Duplicate of this bug: 423265
Comment on attachment 305237 [details] [diff] [review]
Patch (v1)

Trying to get a review from gavin.
Attachment #305237 - Flags: review?(mano) → review?(gavin.sharp)
Whiteboard: [has patch] [needs review gavin]
Target Milestone: Firefox 3 beta4 → Firefox 3
Whiteboard: [has patch] [needs review gavin] → [has patch] [needs review gavin][RC2?]
There's no need to take this for RC2.
Whiteboard: [has patch] [needs review gavin][RC2?]
Attachment #305237 - Flags: review?(gavin.sharp) → review+
Flags: wanted1.9.0.x?
Target Milestone: Firefox 3 → Firefox 3.1
Target Milestone: Firefox 3.1 → Firefox 3
Ready to land it on mozilla-central.
Keywords: checkin-needed
Comment on attachment 305237 [details] [diff] [review]
Patch (v1)

I'm not quite sure whether this is desired for the branch, but I think it's worth giving a try.
Attachment #305237 - Flags: approval1.9.0.1?
Attachment #305237 - Flags: approval1.9.0.1? → approval1.9.0.2?
http://hg.mozilla.org/index.cgi/mozilla-central/rev/e30d33fe2ed8
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: Firefox 3 → Firefox 3.1a1
Attached image need tooltip
This is good.
But, feed link end is hidden.
I feel that feed link need link tooltip.
(In reply to comment #13)
> This is good.
> But, feed link end is hidden.
> I feel that feed link need link tooltip.

Would you mind filing a new bug for this and CCing me on that?
(In reply to comment #14)
> (In reply to comment #13)
> > This is good.
> > But, feed link end is hidden.
> > I feel that feed link need link tooltip.
> 
> Would you mind filing a new bug for this and CCing me on that?
> 
filed Bug 444878

Depends on: 444878
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a1pre) Gecko/2008071504 Minefield/3.1a1pre as well as the equivalent nightly on Mac. I verified using the RSS feed for New Bugs in Bugzilla.
Status: RESOLVED → VERIFIED
Attached image page info (trunk build)
I still see this bug on some (but not all) saved searches here in bugzilla, see attached screenshot. "Bugs Filed Today" on the bugzilla front page works, the saved search for "My Bugs" does not. One example where I still see the problem:

https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&bug_status=UNCONFIRMED&emailassigned_to1=1&emailtype1=exact&email1=&emailassigned_to2=1&emailreporter2=1&emailqa_contact2=1&emailtype2=exact&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=3d&chfieldto=Now&chfield=[Bug+creation]&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&known_name=Recent+Unconfirmed&query_based_on=Recent+Unconfirmed&field0-0-0=noop&type0-0-0=noop&value0-0-0=

At least the "Subscribe" button is now always visible without scrolling, but the scroll bar is still present sometimes.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a1pre) Gecko/2008071502 Minefield/3.1a1pre ID:2008071502
I see what Elmar notes in Comment 17 with the longer query - apparently the "Bugs Filed Today" Query was not quite long enough to see the issue. When I use the query in Comment 17 in Windows Vista and Mac, I see the scrollbar at the bottom. This is using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a1pre) Gecko/2008071502 Minefield/3.1a1pre and Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a1pre) Gecko/2008071504 Minefield/3.1a1pre.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
This instance of the problem can be fixed by increasing the flex attribute of the spacer element in the patch (attachment 305237 [details] [diff] [review]), but I guess no matter how much we increase this value, longer feed URIs can be constructed to cause the scrollbar to appear.  Is there a better way of doing this via some other XUL layout mechanism?
Comment on attachment 305237 [details] [diff] [review]
Patch (v1)

Please request approval on a patch that has landed on mozilla-central and fully fixed this bug. Since the bug was reopened, that doesn't appear to be the case.
Attachment #305237 - Flags: approval1.9.0.2?
(In reply to comment #19)
>Is there a better way of doing this via some other XUL layout mechanism?
I can't remember for sure but try wrapping the label in a second vbox i.e.
<vbox>
  <vbox align="start">
    <hbox>
      <label flex="1" crop="center".../>
    </hbox>
  </vbox>
</vbox>
Because the inner vbox has align="start" the hbox will only attempt to consume the preferred width of the label however as the label text length increases its preferred width increases until the vbox can take no more and then as it is flexible the label should then begin to crop. At least, that's the theory!
Attached patch Followup patchSplinter Review
(In reply to comment #21)
> I can't remember for sure but try wrapping the label in a second vbox i.e.

Neil, thanks for the nice explanation.  In addition to a working patch without a spacer hack, I now have a better understanding of XUL box layout...  :-)
Attachment #334560 - Flags: review?(gavin.sharp)
Attachment #334560 - Flags: review?(gavin.sharp) → review+
The followup patch (attachment 334560 [details] [diff] [review]) needs to be checked in on the trunk.
Keywords: checkin-needed
Pushed as 17113:5e9fa23aef29.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Doesn't really meet the "wanted" criteria (security, stability, regression from maintenance release).
Flags: wanted1.9.0.x?
Verified fixed in today's nightly builds.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.