Closed
Bug 745530
Opened 12 years ago
Closed 12 years ago
"ASSERTION: can't scroll to empty anchor name" with null char
Categories
(Core :: DOM: Navigation, defect)
Core
DOM: Navigation
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: jruderman, Assigned: torisugari)
References
Details
(Keywords: assertion, testcase)
Attachments
(3 files, 3 obsolete files)
###!!! ASSERTION: can't scroll to empty anchor name: '!aScroll', file layout/base/nsPresShell.cpp, line 2953
Reporter | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
The raw string is "#\x00" nsStandardURL (probably) converts it to "#%00" Before calling nsUnescape(...), ref does exist. '#' + "%00" After calling nsUnescape(...), ref is empty string. '#' + '/0' And utf-8 mode is checking whether it's empty or not. The problem is charset-conversion mode. So all we should do to fix this bug is add one more empty string check. The remaining issues are... 1. Charset conversion is really required? IIRC, regardless of document charset, URIs don't copy raw strings any longer. It should always escaped UTF-8. 2. Bug 308590 is fixed. It's time to switch to nsIURI::GetRef()? (bug 126432)
Assignee | ||
Comment 3•12 years ago
|
||
By the way, I don't understand why this bug can block other bugs, for PresShell::GoToAnchor() catches the error successfully. Doesn't it?
Attachment #620674 -
Flags: review?(justin.lebar+bug)
Assignee | ||
Comment 4•12 years ago
|
||
Sorry that was wrong file.
Attachment #620674 -
Attachment is obsolete: true
Attachment #620674 -
Flags: review?(justin.lebar+bug)
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #620675 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #620677 -
Flags: review?(justin.lebar+bug)
Comment 6•12 years ago
|
||
> By the way, I don't understand why this bug can block other bugs, Yeah, I was confused too. I'm don't think I'm not spoiling any secrets by saying that bug 343943 is a metabug for a fuzzer.
Comment 7•12 years ago
|
||
I'd like bz to look at this, just to be safe. > + // When newHashName contains "%00", unescaped string may be empty. Please elaborate, along the lines of "and GoToAnchor asserts if we ask it to scroll to an empty ref." > + scroll &= (!uStr.IsEmpty()); So there's no question about datatypes and bitwise &, would you mind changing this to |scroll = scroll && !uStr.IsEmpty()|?
Comment 8•12 years ago
|
||
Comment on attachment 620677 [details] [diff] [review] simple fix v3 And by "bz" I meant "a docshell peer". :)
Attachment #620677 -
Flags: review?(justin.lebar+bug)
Attachment #620677 -
Flags: review?(bugs)
Attachment #620677 -
Flags: feedback+
Comment 9•12 years ago
|
||
Comment on attachment 620677 [details] [diff] [review] simple fix v3 r=me, but make the changes Justin suggested.
Attachment #620677 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 10•12 years ago
|
||
Addressed comment #7 an comment #9 > bug 343943 is a metabug for a fuzzer. Aha, that makes sense. That is, the goal or this bug is merely shut up the assertion and nothing else.
Attachment #620677 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #620758 -
Flags: review?(bugs)
Updated•12 years ago
|
Attachment #620758 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•12 years ago
|
Comment 11•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/48a5cc532b2b Should this have a crashtest?
Assignee: nobody → torisugari
Flags: in-testsuite?
Keywords: checkin-needed
Target Milestone: --- → mozilla15
Comment 12•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/48a5cc532b2b
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•