Closed
Bug 535530
Opened 15 years ago
Closed 13 years ago
View Source highlighting of entities off by one; When a semicolon has been tokenized in a character reference, should return to the return state eagerly
Categories
(Core :: DOM: HTML Parser, defect, P3)
Core
DOM: HTML Parser
Tracking
()
RESOLVED
FIXED
mozilla11
Tracking | Status | |
---|---|---|
firefox10 | --- | wontfix |
People
(Reporter: hsivonen, Assigned: hsivonen)
References
Details
Attachments
(1 file)
(deleted),
patch
|
smaug
:
review+
christian
:
approval-mozilla-aurora-
|
Details | Diff | Splinter Review |
Spinoff from bug 535499: If document.write ends with the semicolon of a named character reference, the tokenizer state is the data state when returning to the parser thread. (Instead, the return state is the data state and the next input character will cause the return to the data state.) This causes a useless speculation failure. Should move to the return state eagerly to put the tokenizer into the data state in time for the state transfer over the thread boundary.
Assignee | ||
Updated•15 years ago
|
Priority: P2 → P3
Could you please update the summary and also add comment that this also causes one more character after semicolor highlighted as part the preceding HTML entity reference. You linked to this bug in http://hsivonen.iki.fi/view-source/ . But people not reading that article will file bugs against it and will never find this bug.
Assignee | ||
Updated•13 years ago
|
Summary: [HTML5] When a semicolon has been tokenized in a character reference, should return to the return state eagerly → View Source highlighting of entities off by one; When a semicolon has been tokenized in a character reference, should return to the return state eagerly
Comment 2•13 years ago
|
||
(In reply to :aceman from comment #1) > Could you please update the summary and also add comment that this also > causes one more character after semicolor highlighted as part the preceding > HTML entity reference. You linked to this bug in > http://hsivonen.iki.fi/view-source/ . But people not reading that article > will file bugs against it and will never find this bug. Well if that IS in fact the same bug as this, then the importance is completely wrong. Fixing incorrect highlighting is NOT an enhancement.
Assignee | ||
Comment 3•13 years ago
|
||
(In reply to Bill Gianopoulos from comment #2) > Well if that IS in fact the same bug as this, then the importance is > completely wrong. Fixing incorrect highlighting is NOT an enhancement. Good point.
Severity: enhancement → normal
Comment 4•13 years ago
|
||
It occurs to me that this might be confusing for those not familiar with US-English marketing-speak. "Enhancement" in software bug reporting context means "Request for a new feature". Therefore, it is not appropriate for fixing an existing feature.
Comment 5•13 years ago
|
||
Probably related: bug 701775. Not a dupe since current bug (535530) was reported far before HTML5 parser has been first used for View Source.
Assignee | ||
Updated•13 years ago
|
Assignee | ||
Comment 7•13 years ago
|
||
Assignee | ||
Comment 8•13 years ago
|
||
Comment on attachment 576479 [details] [diff] [review] Transition eagerly when a semicolon matches This patch first fixes a major brainfart that turns out not to have much impact for Gecko: it moves the setting of the reconsume variable to immediately before the calls to transition() so that transition() sees the right value for reconsume. Then it fixes the actual problem by breaking the named character reference matching loop early when the semicolon is the last character that matches. Normally, the loop is broken by the first character that doesn't match, because HTML requires longest match. However, since the semicolon never appears in the middle of a named character name, there can't be a longer match after a semicolon has been seen. When the loop is broken early, there's no need to reconsume the semicolon and this needs to be taken into account when setting the start of the next text run when applicable.
Attachment #576479 -
Flags: review?(bugs)
Updated•13 years ago
|
Attachment #576479 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 10•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb1badf94821
Comment 11•13 years ago
|
||
Looks like you're leaking on OSX Crashtests: https://tbpl.mozilla.org/php/getParsedLog.php?id=7657775&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=7658113&tree=Mozilla-Inbound
Assignee | ||
Comment 12•13 years ago
|
||
(In reply to Ms2ger from comment #11) > Looks like you're leaking on OSX Crashtests: > > https://tbpl.mozilla.org/php/getParsedLog.php?id=7657775&tree=Mozilla-Inbound > https://tbpl.mozilla.org/php/getParsedLog.php?id=7658113&tree=Mozilla-Inbound Must be bug 699365. That bug added a crashtest. *This* bug neither adds a crashtest nor changes anything that affects refcounting.
Comment 13•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/eb1badf94821
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 14•13 years ago
|
||
Comment on attachment 576479 [details] [diff] [review] Transition eagerly when a semicolon matches (This comment has been posted on all the bugs mentioned in this comment, except bug 710142, so that the release drivers see it regardless of the order in which they process approval requests.) The new View Source implementation landed before Firefox 10 moved to Aurora. Afterwards, a bunch of regressions were identified. Many of the regression fixes didn't land before Firefox 10 moved to Aurora but they have now been fixed on trunk except for bug 710142. To avoid shipping with regressions, we either need to land all the regression fixes on Aurora for Firefox 10 (followed by a fix for bug 710142 in Beta if it doesn't make it before Dec 20th) or switch back to the old View Source implementation on Aurora. The new View Source implementation provides much better diagnostics for Web developers than the old View Source implementation. So I'd like to ask the release drivers to either approve bug 535530, bug 699356, bug 699365, bug 700034, bug 700361, bug 703965, bug 704667 and bug 705473 plus bug 695640, which is a non-regression tweak, or to approve bug 710175 for reverting to the old View Source implementation for Firefox 10.
Attachment #576479 -
Flags: approval-mozilla-aurora?
Comment 15•13 years ago
|
||
Comment on attachment 576479 [details] [diff] [review] Transition eagerly when a semicolon matches [triage comment] We decided to back out the new view source parser (bug 710175) rather than take this fixup for Firefox 10. Denying for Aurora.
Attachment #576479 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Assignee | ||
Comment 16•13 years ago
|
||
Firefox 10 no longer affected for View Source due to bug 710175 landing. WONTFIX for Firefox 10 as far as speculative parsing goes.
You need to log in
before you can comment on or make changes to this bug.
Description
•