View source uses an incorrect font for some portions of the source (related to invalid comments)

RESOLVED FIXED in mozilla11

Status

()

Toolkit
View Source
--
minor
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: MattN, Assigned: hsivonen)

Tracking

({regression})

unspecified
mozilla11
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox10 unaffected)

Details

(URL)

Attachments

(2 attachments)

Created attachment 576330 [details]
Reduced test case

Steps to reproduce:
1) Open reduced test case page in the browser
2) View source of the page

Expected Result:
A consistent font face for all of the source

Actual Result:
Some of the page source is displayed in a different font.

Possibly caused by 482921 or the HTML5 parser itself which seems to handle the conditional comment differently than Firefox 8.0.1

I had a hard time making a reduced test case because the bug only shows up if there are a certain number of characters in the source.  Even deleting on character could cause the problem to go away.  That is why I put many spaces in the test file to make sure it was over the threshold that would cause this bug. To see for yourself, delete all of the spaces/blank lines below my comment and you will see that the problem goes away.
(Assignee)

Updated

7 years ago
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
(Assignee)

Comment 1

7 years ago
The most immediate problem here is the end of one pre and the start of another one. The interesting question is why the preconditions of the code that ends the pre are wrong. See also bug 695640.
See Also: → bug 695640
(Assignee)

Comment 2

7 years ago
Created attachment 577219 [details] [diff] [review]
Take transitions from the comment start state to the data state into account

The problem was that <!--> did a transition from the comment start state to the data state, but the highlighter didn't take that transition into account. I must have missed that transition when I checked which comment states can transition to the data state.
Attachment #577219 - Flags: review?(bugs)

Updated

7 years ago
Attachment #577219 - Flags: review?(bugs) → review+
(Assignee)

Comment 3

7 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/54f41501ef89
status-firefox10: --- → affected
Flags: in-testsuite+
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/54f41501ef89
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Comment 5

6 years ago
Comment on attachment 577219 [details] [diff] [review]
Take transitions from the comment start state to the data state into account

(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 #577219 - Flags: approval-mozilla-aurora?

Comment 6

6 years ago
Comment on attachment 577219 [details] [diff] [review]
Take transitions from the comment start state to the data state into account

[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 #577219 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
(Assignee)

Comment 7

6 years ago
Firefox 10 no longer affected due to bug 710175 landing.
status-firefox10: affected → unaffected
You need to log in before you can comment on or make changes to this bug.