411213 - More issues with rtl, -moz-column, overflow: -moz-hidden-unscrollable

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Jesse Ruderman]

The testcases are similar to the testcase in bug 406380, but they still cause problems on trunk.

Comment 1

[Jesse Ruderman]

Attachment: testcase 1 (might crash Firefox when loaded)

###!!! ASSERTION: Attempting to allocate excessively large array: 'Error', file nsTArray.cpp, line 66

I'm guessing this crashes.  (In my tree, I have made this assertion abort.)

Comment 2

[Jesse Ruderman]

Attachment: testcase 2

###!!! ASSERTION: integer overflow: 'mMaxTextLength <= mMaxTextLength + aFrame->GetContentLength()', file /Users/jruderman/trunk/mozilla/layout/generic/nsTextFrameThebes.cpp, line 1081

###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file /Users/jruderman/trunk/mozilla/gfx/thebes/src/gfxSkipChars.cpp, line 92

This testcase does not trigger a crash or abort.

Comment 3

[Daniel Veditz [:dveditz]]

In a windows debug build testcase 1 triggered a DEP error that shut down Firefox. Didn't see a problem with either testcase in a 1.8 branch debug build.

Comment 4

[Daniel Veditz [:dveditz]]

Tentative P1 because it blocks further fuzzing.

Who is an appropriate owner?

Comment 5

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

Simon.

Comment 6

[David Baron :dbaron:]

Is overflow:-moz-hidden-unscrollable really needed here?  If so, it probably shouldn't be too hard to figure out, since there are relatively few codepaths where that causes different behavior from overflow:visible.  (We could potentially even limit it to trusted style sheets only.)

Comment 7

[Jesse Ruderman]

Yes, the testcase needs overflow:-moz-hidden-unscrollable.  It doesn't crash with any of the standard overflow values (visible, hidden, scroll, auto).

Comment 8

[David Baron :dbaron:]

Attachment: patch

I did the code audit I suggested in comment 6, and found one very suspicious piece of code.  Removing it happens to fix the bug.

The idea of overflow:-moz-hidden-unscrollable is that it *doesn't* affect layout.  So best not to change Reflow methods based on it (except overflow area computation).

Comment 9

[David Baron :dbaron:]

(And, to be clear, I'm working on this non-blocking1.9+ bug because Jesse asked me to have a quick look since it blocks his fuzzing work.)

Comment 10

[David Baron :dbaron:]

Comment on attachment 308783 [details] [diff] [review]
patch

Simple patch to make an rarely-used mostly-obsolete -moz-* value not change layout algorithms in unexpected and crashy ways.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 308783 [details] [diff] [review]
patch

a1.9=beltzner

Comment 12

[David Baron :dbaron:]

Fix checked in to trunk, 2008-03-13 08:23 -0700.

Comment 13

[Samuel Sidler (old account; do not CC)]

This landed in March and doesn't apply to the 1.8 branch. Any reason not to open this bug up?