Closed Bug 511482 Opened 15 years ago Closed 15 years ago

moz-column-width can freeze the browser

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.3a1
Tracking Flags:
Tracking Status
status1.9.2 --- beta1-fixed
status1.9.1 --- .4-fixed

People

(Reporter: vingtetun, Assigned: MatsPalmgren_bugz)

Details

(4 keywords)

Attachments

(2 files)

testcase
2.56 KB, text/html
Details
Patch rev. 1
5.38 KB, patch
dbaron
: review+
dbaron
: approval1.9.2+
dveditz
: approval1.9.1.4+
dveditz
: approval1.9.0.15+
Details | Diff | Splinter Review
Attached file testcase — 
See the attached url to reproduce/freeze your browser.

There is no javascript at all in it, only html. It looks like the browser enter in an infinite loop, and if I comment the last div element all works fine.
Mozilla/5.0 (X11; U; Linux i686; rv:1.9.1.3pre) Gecko/20090819 SeaMonkey/2.0b2pre

I can reproduce this, also on WinXP SP3. Note that the test case has the <html> </html> tags reversed, but the issue occurs even when the tags are in the correct order.
Attached patch Patch rev. 1 — — Splinter Review 
PR_MAX evaluates its args twice, use NS_MAX instead.
Assignee: nobody → matspal
Attachment #395750 - Flags: review?(fantasai.bugs)
I will file a bug to fix that for all of layout/ unless there is one already.
Why don't we just fix the PR_MAX macro?
I thought we were moving away from NSPR in general?  I've been told
in the past to use fabs() instead of PR_ABS for example.
(bug 410917 comment 16)

Also, the PR_* macros needs to work for plain C, but I guess that can
be fixed with having template versions under #ifdef __cplusplus,
but is having different behaviour between C / C++ versions acceptable?
(It can be solved with gcc using a ({ ... }) expression, but I don't
think there is a general C solution to the problem).
Comment on attachment 395750 [details] [diff] [review]
Patch rev. 1

I guess that makes sense. :) I'm going to defer to the module peers on this, though, because I haven't seen NS_MAX used in layout/ before.
Attachment #395750 - Flags: review?(fantasai.bugs)
http://hg.mozilla.org/mozilla-central/rev/b59ba655c72d
Status: NEW → RESOLVED
Closed: 15 years ago
OS: Linux → All
Priority: -- → P3
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
I filed bug 512106 to follow up the discussion on switching to
NS_MIN/MAX everywhere.
Attachment #395750 - Flags: approval1.9.1.3?
Attachment #395750 - Flags: approval1.9.0.14?
Attachment #395750 - Flags: approval1.9.1.4?
Attachment #395750 - Flags: approval1.9.1.3?
Attachment #395750 - Flags: approval1.9.0.15?
Attachment #395750 - Flags: approval1.9.0.14?
Comment on attachment 395750 [details] [diff] [review]
Patch rev. 1

Pushing out approval requests.
Keywords: hang
Comment on attachment 395750 [details] [diff] [review]
Patch rev. 1

Approved for 1.9.1.4 and 1.9.0.15, a=dveditz for release-drivers
Attachment #395750 - Flags: approval1.9.1.4?
Attachment #395750 - Flags: approval1.9.1.4+
Attachment #395750 - Flags: approval1.9.0.15?
Attachment #395750 - Flags: approval1.9.0.15+
Fixed on 1.9.0 branch, aka CVS HEAD, for 1.9.0.15:
mozilla/layout/base/nsLayoutUtils.cpp 	3.146
mozilla/layout/generic/crashtests/511482.html 	1.1
mozilla/layout/generic/crashtests/crashtests.list 	1.127
Keywords: fixed1.9.0.15
Verified for 1.9.0.15 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.15pre) Gecko/2009091606 GranParadiso/3.0.15pre (.NET CLR 3.5.30729) using attached testcase.
Keywords: fixed1.9.0.15 → verified1.9.0.15
Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.4pre) Gecko/20090930 Shiretoko/3.5.4pre using attached testcase.
Keywords: verified1.9.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: