Last Comment Bug 652775 - text in <legend> doesn't wrap
: text in <legend> doesn't wrap
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Layout: Form Controls (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla6
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
http://moodle.jonathan-list.com/mod/l...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-26 04:14 PDT by JSL
Modified: 2011-07-29 01:03 PDT (History)
7 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
some examples (659 bytes, text/html; charset=UTF-8)
2011-04-26 10:04 PDT, David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch)
no flags Details
Remove the default nowrap styling on <legend>. (2.12 KB, patch)
2011-05-07 20:21 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Splinter Review
Remove the default nowrap styling on <legend>. (2.54 KB, patch)
2011-05-08 05:19 PDT, Boris Zbarsky [:bz]
dbaron: review+
Details | Diff | Splinter Review

Description JSL 2011-04-26 04:14:46 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; en-US) AppleWebKit/534.16 (KHTML, like Gecko) Chrome/10.0.648.205 Safari/534.16
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0

I tested this on my Mac, OSX and Ubuntu 11-4 Laptops. In both instances, Firefox does not correctly render the page.  This is a critical bug, because I will have many remote users coming to this lesson to engage the material.  If Firefox does not render the pages appropriately, I will be forced to warn the students that they cannot complete the lesson using FF, and should instead use Chrome or Safari.

Reproducible: Always

Steps to Reproduce:
1. Go to: http://moodle.jonathan-list.com/mod/lesson/view.php?id=4&pageid=19
2. Log in as guest and load a question page (that link should take you there)
3. If it says "Start from beginning?" Say no.

Actual Results:  
The body of content related to the question is rendered in one long line.

Expected Results:  
Should be loaded within the contents of the div.
Comment 1 Robert Longson 2011-04-26 06:38:18 PDT
You have the div with the text in as a child of a legend element which is a child of a fieldset. I think you want the legend/fieldset tags only around the possible answers and not the question.
Comment 2 JSL 2011-04-26 06:41:19 PDT
(In reply to comment #1)
> You have the div with the text in as a child of a legend element which is a
> child of a fieldset. I think you want the legend/fieldset tags only around the
> possible answers and not the question.

Yes, the problem may be with the code on the site, however since it renders fine in MSIE, Chrome and Safari, this is an issue with FF 4.  The other problem is that I'm not a Moodle Developer; I'm just a teacher in this instance.  I'm not going to hack around in the Moodle Core to fix a problem that is only showing up in FF.
Comment 3 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-04-26 09:23:08 PDT
So the issue is that the text that starts with "This is a little bit of a puzzle" doesn't wrap?  (I was looking at the other text, and very nearly posted a comment saying it worked fine for me.)
Comment 4 Boris Zbarsky [:bz] 2011-04-26 09:32:04 PDT
This is more or less purposeful.  Legends are styled "white-space: nowrap" in our UA stylesheet.  Last I checked, that was for compat reasons....  If those are no longer operative, we can probably remove that.
Comment 5 Boris Zbarsky [:bz] 2011-04-26 09:33:43 PDT
In the meantime, if you have any control at all over the CSS in question you can override the style in your stylesheet by setting:

  legend { white-space: normal }

But it sounds like you may not...
Comment 6 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-04-26 09:57:59 PDT
FWIW, in 3.6 the content that's in the legend in 4.0 isn't in the legend at all, so the trigger for this bug may have been the HTML5 parser.  (I wonder which way it parses in other browsers.)

That said, the source of the page is:

		<legend class="ftoggler"><div class="box contents"><div style="width: 600px;">This is a little bit of a puzzle.  Below you will find four links.  Select the answer that matches the link with the title that most accurately matches the associated URL.  Please use the skills you learn on the last page to complete this task; it will make your life much simpler.</div>

Maybe the "nowrap" that other browsers do is really something that doesn't override a width on a child of the legend... i.e., just a difference in width calculation.  (Is that *really* needed for compat?)
Comment 7 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-04-26 10:04:43 PDT
Created attachment 528334 [details]
some examples
Comment 8 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-04-26 10:05:54 PDT
Chromium seems to wrap both examples; haven't tried other browsers yet.
Comment 9 JSL 2011-04-26 10:20:43 PDT
in Reply to Comment 6:

the HTML: <legend class="ftoggler"><div class="box contents"><div style="width: 600px;">This is a little bit of a puzzle.  Below you will find four links.  Select the answer that matches the link with the title that most accurately matches the associated URL.  Please use the skills you learn on the last page to complete this task; it will make your life much simpler.</div>

Shows up in Chrome, exactly like you see.  The part, "div style = "width 600px;">"  is actually added in the content; I was hoping to force FF4 into rendering the box correctly as a workaround.  

The same code is displayed in the source page for FF4.
Comment 10 Boris Zbarsky [:bz] 2011-04-26 10:38:14 PDT
JSL, if you can add inline styles, you can just add the style from comment 5 to your <legend> to make things work for you.

David, on your testcase I see WebKit and opera wrapping both sets of text.  IE9 wraps the second, but not the first.  We could sort of get that behavior by styling |fieldset > legend > * { white-space: normal; }| in forms.css, but that would be sort of sad.... and possibly override other styles that set white-space in ua.css.  I'd rather just remove the white-space styling at all than do that....

Alternately, we could add a new value of white-space which is treated as "nowrap" but inherits as "normal"... except to text nodes or something.  It's a mess.
Comment 11 JSL 2011-04-26 10:40:48 PDT
(In reply to comment #10)
> JSL, if you can add inline styles, you can just add the style from comment 5 to
> your <legend> to make things work for you.

I have at least 50 (but I think close to 100) pages that this affects.  While I *could* add this to each bit of text, I don't really have time to do it right now.
Comment 12 Boris Zbarsky [:bz] 2011-04-26 10:43:39 PDT
To be clear, I'm offering you a workaround you can use for the problem in the interim, since the earliest that you will see a fix for this issue shipping is in August.  Whether you want to make use of the workaround is up to you.
Comment 13 JSL 2011-04-26 10:52:07 PDT
(In reply to comment #12)
> To be clear, I'm offering you a workaround you can use for the problem in the
> interim, since the earliest that you will see a fix for this issue shipping is
> in August.  Whether you want to make use of the workaround is up to you.

And I appreciate it; really I do! It's the end of the semester, though, and I have tons of things that are higher priority...

And I feel happy that I found a bug to be fixed so that FF 5 will be better! :-)  I probably will do the work-around after the semester is over: I just have lit reviews, journal articles, an entire web app and 30 students worth of grading to finish! :-)
Comment 14 Boris Zbarsky [:bz] 2011-04-26 10:55:03 PDT
Oh, I understand how the end of semester thing works.  :)

This won't be fixed in Firefox 5; that's shipping in June.  But hopefully in Firefox 6 (August).
Comment 15 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-04-26 11:43:48 PDT
Why can't we just match WebKit and Opera, drop the 'white-space: normal', and make any other layout changes we need to match?
Comment 16 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-04-26 11:44:20 PDT
er, drop the white-space: nowrap.  (They're both 6 letters starting with n!)
Comment 17 Boris Zbarsky [:bz] 2011-04-26 11:58:42 PDT
I'd be fine with just dropping the white-space style, I think.

We shouldn't need any changes other than perhaps to tests to accomodate that, I think....  we'll just have different behavior.
Comment 18 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-04-26 12:13:41 PDT
We should check that we pass an appropriate available width in so that the shrink-wrapping works correctly.
Comment 19 Boris Zbarsky [:bz] 2011-04-26 12:14:55 PDT
Sure, but I'm pretty sure we do.

Are you writing the patch, or should I?
Comment 20 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-04-26 17:31:07 PDT
Could you?
Comment 21 Boris Zbarsky [:bz] 2011-04-26 21:04:20 PDT
Will do.
Comment 22 Boris Zbarsky [:bz] 2011-05-07 20:21:20 PDT
Created attachment 530891 [details] [diff] [review]
Remove the default nowrap styling on <legend>.
Comment 23 Boris Zbarsky [:bz] 2011-05-08 05:19:00 PDT
Created attachment 530913 [details] [diff] [review]
Remove the default nowrap styling on <legend>.

This passes try
Comment 24 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-05-08 05:28:57 PDT
Comment on attachment 530913 [details] [diff] [review]
Remove the default nowrap styling on <legend>.

r=dbaron
Comment 25 Boris Zbarsky [:bz] 2011-05-11 19:03:33 PDT
http://hg.mozilla.org/mozilla-central/rev/0191efa548ac
Comment 26 Simona B [:simonab] 2011-07-28 08:40:17 PDT
Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0

Verified issue on Windows XP, Windows 7, Mac OS X 10.6 and Ubuntu using the examples from Comment 7 (looks good).

But when I verify the issue using the STR from Comment 0, the first row <A) Learn more about Moodle> is a little out of the rectangle (especially on Ubuntu). Is that ok?
Comment 27 Boris Zbarsky [:bz] 2011-07-28 08:51:26 PDT
That's correct for a legend, which sits with its first line centered on the top border, yes.
Comment 28 Simona B [:simonab] 2011-07-29 01:03:25 PDT
Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0

Based on Comment 27, setting resolution of this issue to VERIFIED FIXED.

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