Created attachment 615136 [details] testcase ###!!! ASSERTION: can't scroll to empty anchor name: '!aScroll', file layout/base/nsPresShell.cpp, line 2953
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)
Created attachment 620674 [details] [diff] [review] simple fix By the way, I don't understand why this bug can block other bugs, for PresShell::GoToAnchor() catches the error successfully. Doesn't it?
Created attachment 620675 [details] simple fix v2 Sorry that was wrong file.
Created attachment 620677 [details] [diff] [review] simple fix v3
> 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.
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 on attachment 620677 [details] [diff] [review] simple fix v3 And by "bz" I meant "a docshell peer". :)
Comment on attachment 620677 [details] [diff] [review] simple fix v3 r=me, but make the changes Justin suggested.
Created attachment 620758 [details] [diff] [review] simple fix v4 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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/48a5cc532b2b Should this have a crashtest?