Closed Bug 315933 Opened 16 years ago Closed 16 years ago

The slash in empty element tags in XML are red in view source

Categories

(Core :: DOM: HTML Parser, defect, P3)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: zcorpan, Assigned: mrbkap)

References

Details

(Keywords: regression, verified1.8.0.10, verified1.8.1.2, Whiteboard: regression from 314980)

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051109 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051109 Firefox/1.6a1

View source of the document data:application/xml,<foo/> shows the slash in red, as if it were a syntax error. It is not a syntax error in XML, therefore the slash should not be red. (It is a syntax error in text/html documents though; please leave it red for view-source:data:text/html,<foo/> !)

Reproducible: Always

Steps to Reproduce:
1. Open view-source:data:application/xml,<foo/>
2.
3.

Actual Results:  
The slash is red.

Expected Results:  
The slash should be the same color as the angle brackets (black).
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051110 Firefox/1.6a1

->HTML: Parser (syntax highlighting bugs seem to be spread around, so I apologize in advance for guessing where to route this...)
Assignee: nobody → mrbkap
Status: UNCONFIRMED → NEW
Component: View Source → HTML: Parser
Ever confirmed: true
Product: Firefox → Core
QA Contact: view.source → parser
Version: unspecified → Trunk
D'oh! I realized this yesterday but forgot to file the bug. I'll fix this soon.
Blocks: 314980
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.9alpha
Attached patch Potential patch (obsolete) — Splinter Review
I haven't compiled or tested this yet, but I think it should do the right thing.
Attached patch The simple routeSplinter Review
Getting the error reporting right in all cases in view-source is more work than is worth it (and that's what our XML parser is for, anyway), so simply don't bother coloring trailing /s in XML.
Attachment #202620 - Attachment is obsolete: true
Attachment #202703 - Flags: superreview?(jst)
Attachment #202703 - Flags: review?(jst)
Comment on attachment 202703 [details] [diff] [review]
The simple route

r+sr=jst
Attachment #202703 - Flags: superreview?(jst)
Attachment #202703 - Flags: superreview+
Attachment #202703 - Flags: review?(jst)
Attachment #202703 - Flags: review+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Why not the simpler

+            if (kQuote == aChar || kApostrophe == aChar ||
+                (kForwardSlash == aChar && !(aFlag & NS_IPARSER_FLAG_XML))) {
               mInError = PR_TRUE;
We still need to fall through to the ConsumeInvalidAttribute below, which I don't think your patch allows.
Ah, of course. /me needs to get that tunnel vision fixed
The slash is still red in View Selection Source.

Steps to reproduce:
1. Open data:application/xml,<p xmlns="http://www.w3.org/1999/xhtml">foo<br/>bar</p>
2. Select All
3. View Selection Source
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I believe that was intentional since the slash is in fact an error.
That's a different bug. The view selection source code needs to tell the parser that it's parsing XML. This bug is FIXED.
Status: REOPENED → RESOLVED
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
Take on branches if we take bug 314980
Flags: blocking1.8.1.2?
Flags: blocking1.8.0.10?
Whiteboard: regression from 314980
(In reply to comment #13)
> Take on branches if we take bug 314980

Dan, the branch patch in bug 314980 includes the patch for this bug.
Flags: blocking1.8.1.2?
Flags: blocking1.8.1.2+
Flags: blocking1.8.0.10?
Flags: blocking1.8.0.10+
Comment on attachment 202703 [details] [diff] [review]
The simple route

approved for 1.8/1.8.0 branches, a=dveditz for drivers
Attachment #202703 - Flags: approval1.8.1.2+
Attachment #202703 - Flags: approval1.8.0.10+
Keywords: regression
This was fixed on the 1.8 branches.
Verified trailing '/' in view-source:data:application/xml,<foo/> is not red for 2.0.0.2pre and 1.5.0.10pre.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.2pre) Gecko/2007011103 BonEcho/2.0.0.2pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.10pre) Gecko/20070111 Firefox/1.5.0.10pre
Hmm. Now that <br/> and <math/> are valid in HTML5 and work for integrating an HTML5 parser in Gecko has started, maybe this bug should be reversed or at least the slash should not be red when it's the trailing slash on a void HTML element or an SVG or MathML element?
The slash redness should probably be relaxed as part of bug 482921.
You need to log in before you can comment on or make changes to this bug.