419531 - Text moves around whilst highlighting

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Steve England [:stevee]]

Attachment: shows the weird behaviour whilst selecting text.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022422 Minefield/3.0b4pre ID:2008022422

NSA showed me some misbehaving html on IRC, and I reduced it to the attached testcase.

1. New profile, start firefox
2. Load testcase.
3. Place the mouse in the middle of the block of text
4. Hold left mouse button
5. Mouse the mouse around

Expected:
- Text gets highlighted as you move the mouse around, but the layout doesn't change

Actual:
- Text gets highlighted as you move the mouse around, but the layout changes; the gap between "This text doesn't move around" and "This text does" keeps disappearing.

Works: 20080215_2302_firefox-3.0b4pre.en-US.win32
Broken: 20080216_0740_N_firefox-3.0b4pre.en-US.win32

Checkins to module PhoenixTinderbox between 2008-02-15 23:02 and 2008-02-16 07:39 : 
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1203145320&maxdate=1203176399

Due to bug 392785 I'd say.

Comment 1

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

Crikey!

Comment 2

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

Attachment: fix

Here's what happens in this bug:

-- we reflow a line with a preformatted tab. This stores the widths of its preformatted tabs in a frame property.

-- later we do an incremental reflow of another line. The first line is not dirty so it doesn't get a proper reflow. It *does* participate in min-width computation for the fieldset. During the computation of the min-width for the textframe containing the tab, we wipe out the saved tab widths and replace them with new widths (all zero). But we can't compute accurate tab widths during min-width computation, so the current code bails out and makes them all zero.

-- later we paint that line. Painting uses the stored tab widths, which are now bogus.

This patch fixes the problem by not touching the frame property during intrinsic width computation. We just return early, returning null which treats all tab widths as zero (which we were doing before, but more clumsily by filling in the array with zeroes).

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 305938 [details] [diff] [review]
fix

You should probably just declare the PRUint32 i in the for-loop initializer.

Also, I really don't understand why you use 0 tab-widths during intrinsic width calculation.  The intrinsic width accumulated so far in the Inline*WidthData is a perfectly good horizontal offset (though maybe not from the right thing, but that should be easy to add).  But that's the topic of another bug.  (Is it filed, at least?)

r+sr=dbaron

Comment 4

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

I don't think that's filed.

Using the intrinsic width as the x-offset would mean we could calculate a preferred size for the text which is less than the actual size at layout time. (It can't make the block preferred size too small, though.) Is that OK?

Comment 5

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

Checked in.

Comment 6

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022702 Minefield/3.0b4pre ID:2008022702
VERIFIED