368175 - [1.8 branch] crash with "direction:rtl" and "-moz-column"

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[hlb]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; zh-tw) AppleWebKit/418.9.1 (KHTML, like Gecko) Safari/419.3
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1) Gecko/20061026 BonEcho/2.0

If there are CSS rules "direction: rtl", nested "-moz-column", and chinese characters ( not tried other unicode characters yet ), it always crashed. Even if there is no nested column, it behaves in a weird way.
example: http://hlb.yichi.org/test/firefox/crash-by-column.html

Reproducible: Always

Steps to Reproduce:
1. open firefox
2. link to http://hlb.yichi.org/test/firefox/crash-by-column3.html
Actual Results:  
crash every time

Comment 1

[Jesse Ruderman]

WFM using Mac trunk.

Comment 2

[Uri Bernstein]

Possibly related to (or duplicate of) bug 288467 / bug 339081.

Comment 3

[hlb]

Somebody reports that Firefox 3a1 does not crash.

Comment 4

[Nicolás (Zona Firefox)]

I tested it on Minefield 3a1 and no crash.

Comment 5

[Gea-Suan Lin]

Test on Windows XP with 2.0.0.2pre (2007020204) will not crash. Tab which opens the url will freeze but can be turn off.

Comment 6

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

Can anyone confirm this is still a branch crasher?

Comment 7

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

It doesn't crash in my Mac 2.0.0.3 build.

Comment 8

[hlb]

It still makesmy Mac 2.0.0.3 build crash.

Comment 9

[JEONG, MunShik]

It still crash Firefox 2.0.0.7/OSX 10.4.10. 

But Works for me under 
 Minefield3.0a9/OSX 10.4.10 

Comment 10

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).

Comment 11

[Daniel.S]

The testcases are misrendered in Fx 2.0.0.20 and eventually crash. They work without anyproblems in Firefox 3 and higher.

Comment 12

[Jeff Walden [:Waldo]]

Daniel, mind creating a crashtest for this while you're here (also for the other bugs you've been triaging of late in Layout)?  Information on how to write reftests is here:

http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/README.txt

Crashtests are just a slight variant of reftests that use the "load" type rather than the "==" or "!=" types.  If you have any questions on how to do this, ask here and people will be more than glad to answer questions.

Comment 13

[Daniel.S]

Attachment: Like this?

This crashes Fx 2.0.0.20 every time.

(In reply to comment #12)
> Daniel, mind creating a crashtest for this while you're here (also for the
> other bugs you've been triaging of late in Layout)?

Not at all. I intended to learn more about these anyway after I'm finished with the remaining unconfirmed Layout bugs.

I hope module peers are ok to ask for review?

Comment 14

[Martijn Wargers (dead)]

Afaik, you don't need to ask for review, you can just check it in.

Comment 15

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

At the beginning you probably should be asking for review, especially for reftests.

In this case, are you sure that it would crash inside the harness?  In the harness, it's being loaded inside an iframe... so would the window.resize still do what you think?  And are you sure it would have an effect before the harness moves on to the next page?

Comment 16

[Daniel.S]

(In reply to comment #15)
> At the beginning you probably should be asking for review, especially for
> reftests.

I'll keep that in mind.

> In this case, are you sure that it would crash inside the harness?  In the
> harness, it's being loaded inside an iframe... so would the window.resize
> still do what you think?  And are you sure it would have an effect before
> the harness moves on to the next page?

The resize looses it's effect within an iframe.

Are the width and height of the iframe defined? I noticed if my own iframe can show the complete testcase it only crashes on user action, while it always crashes if the iframe is smaller than the space needed.

Comment 17

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

800 width x 1000 height.

That said, you could just put an iframe inside the testcase and resize the iframe in the testcase itself.

Comment 18

[Daniel.S]

Attachment: crashtest, v2

(In reply to comment #17)
> That said, you could just put an iframe inside the testcase and resize the
> iframe in the testcase itself.

Ok, that does the trick.

Comment 19

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

Comment on attachment 354450 [details] [diff] [review]
crashtest, v2

r+sr=dbaron (although if you need only the small width and not the resizing, you could probably just use a div)

Comment 20

[Daniel.S]

Attachment: crashtest for checkin

(In reply to comment #19)
> (From update of attachment 354450 [details] [diff] [review])
> r+sr=dbaron (although if you need only the small width and not the resizing,
> you could probably just use a div)

Yaeh, no need for complexity. This works just as fine.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/9f80cbefc185 with that last attachment.