403426 - white-space:nowrap style not working for checkbox label

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect, P2)

Description

[Mathias Tschopp]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111104 Minefield/3.0b2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111104 Minefield/3.0b2pre

The following code doesn't render correctly in the latest nightly. The check box and the label should render on the same line.

<div style="border: 1px solid #000000;width:200px">
<input type="checkbox" id="cb1"><label for ="cb1">short label</label>
<span style="white-space:nowrap;"><input type="checkbox" id="cb2"><label for ="cb2">this is a very long label</label></span>
</div>

Reproducible: Always

Steps to Reproduce:
1. try the above-mentioned html code with a nightly build

Comment 1

[Mathias Tschopp]

Attachment: test case

Comment 2

[Boris Zbarsky [:bzbarsky]]

This regressed between 2007-10-19-02 and 2007-10-20-02.  Range:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=MozillaTinderboxAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-10-19+02&maxdate=2007-10-20+02&cvsroot=%2Fcvsroot

roc, I'm betting this is a regression from bug 393096.  We don't have any whitespace in there... so why are we wrapping?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

The bug here is basically that resetting nsLineBreaker needs to clear mBreakHere and mAfterBreakableSpace. That's simple enough

But there's a dual issue which is that when mBreakHere or mAfterBreakableSpace are set and we Reset the linebreaker, we're actually throwing away a usable line-break before the non-text-frame that caused us to Reset. The reftest in the patch shows this on lines 2 and 3. I'm fixing that by having Reset return whether a break opportunity exists at the end of the text. If it does exist, I'm recording that with a textrun flag and letting the text frame use or record the break opportunity.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Crashed in Tp, backed out.

Comment 6

[jason.thurston]

I was about to report a bug but mine probably is the same as this, here's my example. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007113005 Minefield/3.0b2pre

<div style="width: 120px;">
     <span style="font-size: 7pt;white-space: normal">
          <span style="white-space: nowrap;">
                <input name="f1" value="1" type="checkbox">
                &nbsp;HCM-Horizon Clinicals
           </span>
          <!-- should wrap here but doesn't in ff3-->
          <span style="white-space: nowrap;"><input name="f2" value="2" checked="checked" type="checkbox">
                &nbsp;HLB-Horizon Labs
          </span>
          <!-- should wrap here but doesn't in ff3-->
          <span style="white-space: nowrap;"><input name="f3" value="0" checked="checked" type="checkbox">
                &nbsp;UNK-Undeterminable Org
          </span>
     </span>
</div>

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: updated fix

The crash was caused by
      textRun->SetFlagBits(nsTextFrameUtils::TEXT_HAS_TRAILING_BREAK);
with textRun null because we constructed just a partial textrun that was discarded, just to get state set up to process future textruns correctly. So we just need to skip this call when textRun is null.

I'll reland this shortly.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Real updated fix

The previous patch missed content files :-(

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Relanded

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Relanded

Comment 11

[jason.thurston]

It's better but I don't think it's totally fixed b/c the wrapping logic is different then FF2 or IE.  It seems if there is any room, even one pixel, at the end of a "nowrap" text block then the entire next nowrap text block will be placed outside the surrounding block on the same row and only if the current nowrap text block didn't completely fit in the row will the next nowrap text block be started on the following row.  The behavior I would expect is that when the next nowrap block can not fit on the same row as the current one then it is moved to a new row.  Below is my new test code to demonstrate this.

<div style="height: 100%;width: 75px; background:red">
     <span style="font-size: 7pt;white-space: normal">
          <span style="white-space: nowrap;"><input name="f2" value="2" checked="checked" type="checkbox"/>
                &nbsp;1-aaaaaaaaaa
          </span>
          <!-- should wrap here but doesn't in ff3-->
          <span style="white-space: nowrap;"><input name="f2" value="2" checked="checked" type="checkbox"/>
                &nbsp;2-aaaaaaaaaaa
          </span>
          <!-- should wrap here but doesn't in ff3-->
          <span style="white-space: nowrap;"><input name="f2" value="2" checked="checked" type="checkbox"/>
                &nbsp;3-aaaaaaaaaaaaa
          </span>
          <!-- should wrap here but doesn't in ff3-->
          <span style="white-space: nowrap;"><input name="f2" value="2" checked="checked" type="checkbox"/>
                &nbsp;4-aaaaaaaaaaaaaa
          </span>
          <!-- should wrap here but doesn't in ff3-->
     </span>
</div>

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That sounds like bug 405577. If you agree comment there, otherwise file a new bug and CC me. Thanks.

Comment 13

[Mathias Tschopp]

The patch doesn't fix the whole problem. In the test-case, it seems now that the no-wrap style applies to both checkboxes, and forces the rendering on a single line. This is not how Fx 2.0 or Opera behave (2 checkboxes on 2 different lines) which seems the correct behavior.

Comment 14

[Reed Loden [:reed]]

(In reply to comment #13)
> The patch doesn't fix the whole problem.

Read comment #12 and do what it says.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #11)
(In reply to comment #13)

Those are definitely going to be fixed in 405577, the patch I'm going to put there fixes them.