Closed
Bug 879628
Opened 9 years ago
Closed 9 years ago
Ordered list markers of nested ordered lists corrupted when using generated content
Categories
(Core :: Layout: Block and Inline, defect)
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: lang.support, Assigned: bzbarsky)
Details
Attachments
(4 files)
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:21.0) Gecko/20100101 Firefox/21.0 (Beta/Release) Build ID: 20130512194445 Steps to reproduce: Viewed the following content in firefox: <!DOCTYPE html> <html lang="th"> <head> <meta charset="utf-8" /> <title>Thai numbering test page</title> <style type="text/css"> ol { counter-reset: item; } li { display: block; } li:before { content: counters(item, ".", -moz-thai); content: counters(item, ".", thai); counter-increment: item; padding-right: 1.0em; } </style> </head> <body> <ol> <li> รายการ ๑ <ol> <li> รายการ ๑.๑ <ol> <li> รายการ ๑.๑.๑ <ol> <li> รายการ ๑.๑.๑.๑ <li> รายการ ๑.๑.๑.๒ </ol> <li> รายการ ๑.๑.๒ </ol> <li> รายการ ๑.๒ </ol> <li> รายการ ๒ </ol> </body> </html> Actual results: List markers in first tier list display correctly. Second and subsequent tiers are corrupted displaying mojibake. Similar problem exists with -moz-lao, -moz-maynar list types Expected results: list markers should be identical to numbers listed in each list item, and should display as rendered in chromes/webkit.
Reporter | ||
Comment 1•9 years ago
|
||
Updated•9 years ago
|
Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core
Comment 2•9 years ago
|
||
![]() |
Assignee | |
Comment 3•9 years ago
|
||
Thank you for the excellent testcase!
Assignee: nobody → bzbarsky
Status: UNCONFIRMED → NEW
Component: CSS Parsing and Computation → Layout: Block and Inline
Ever confirmed: true
Whiteboard: [need review]
![]() |
Assignee | |
Comment 4•9 years ago
|
||
Attachment #758650 -
Flags: review?(dholbert)
Comment 5•9 years ago
|
||
This looks good; there's one thing I don't understand, though -- how would 'result' be nonempty (and 'offset' be nonzero)? From skimming this file, it looks like this string will always have been passed (unmodified) by nsBulletFrame::AppendCounterText, ...which received it unmodified from nsBulletFrame::GetListItemText, ...which received it from either nsBulletFrame::GetDesiredSize or from nsBulletFrame::PaintBullet, ...which both declare it as "nsAutoString text;" and don't touch it before their (one) call to GetListItemText.
![]() |
Assignee | |
Comment 6•9 years ago
|
||
The relevant caller of nsBulletFrame::AppendCounterText here is in nsCounterUseNode::GetText, and it will use the same string over and over as it loops over the counters for a counters().
Comment 7•9 years ago
|
||
Um, what ensures that these functions increase the length of the string's buffer, or that they set the string's length data correctly? I think they both have an exploitable buffer overrun, unless I'm missing something.
![]() |
Assignee | |
Comment 8•9 years ago
|
||
I'm not sure I follow... OtherDecimalToText and TamilToText work by appending the number in decimal to the string and then going through and adding a fixed number to each codepoint value in the result. This operation doesn't change the length of the string; that's changed by the DecimalToText call. I guess you could get into trouble in OtherDecimalToText if ordinal < 0 and the AppendASCII in DecimalToText fails to realloc the buffer, but I thought we made strings infallible now.
Comment 9•9 years ago
|
||
I don't follow either; I was going to grant r+, but I'll hold off on that until we better understand what dbaron had in mind with the scenario in comment 7, and how that impacts what this patch should do.
Comment 10•9 years ago
|
||
Sorry, never mind; I was misreading.
Updated•9 years ago
|
Attachment #758650 -
Flags: review?(dholbert) → review+
![]() |
Assignee | |
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d27cb123e9de
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla24
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d27cb123e9de
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•