Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11) Assignee
	Description • 15 years ago

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.

	:aceman
	Comment 1 • 13 years ago

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.

	Bill Gianopoulos [:WG9s]
	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.

	Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11) 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.

	Bill Gianopoulos [:WG9s]
	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.

	Marat Tanalin
	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.

	Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11) 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.

	Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11) Assignee
	Comment 10 • 13 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb1badf94821

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	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

	Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11) 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.

	Marco Bonardo [:mak]
	Comment 13 • 13 years ago

https://hg.mozilla.org/mozilla-central/rev/eb1badf94821

	Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11) 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.

	LegNeato
	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.

	Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11) 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.