Closed Bug 43267 Opened 25 years ago Closed 20 years ago

view source displays attributes with quotes in strange places incorrectly

Categories

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

Product:
Core
Component:
DOM: HTML Parser
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Future

People

(Reporter: jmd, Assigned: mrbkap)

References

Details

(Keywords: testcase)

Attachments

(4 files)

testcase
123 bytes, text/html
Details
patch v1
4.38 KB, patch
bzbarsky
: review+
rbs
: superreview+
Details | Diff | Splinter Review
more testcases
779 bytes, text/html
Details
[screenshot] FYI - parsing of the testcase - before & after
21.48 KB, image/png
Details
In some cases, <frameset ...> being shown wrong. Test case will follow.
OK, looks like this is caused by invalid HTML. Misplaced, unmatched quotes. It's amazing it actually renders. Regardless, view source should have no problems with it. 4.7 view source is fine. The " at the end of frameborder is being cut, and the space after that. 4.7 is displaying the initial " colored as the same color as text, which is probably the best we can hope to do.
Attached file testcase —
Keywords: testcase
--> xp apps. sairuh - any ideas where this really belongs? Perhaps networking? According to the component page, its xp apps gui, but I seem to doubt it is ben's fault.
Assignee: asa → don
Component: Browser-General → XP Apps
QA Contact: doronr → sairuh
here's the source (of the testcase) i see using View Page Source in 4.73 on linux: <html> <frameset "frameborder="0" extra attributes here> <frame src="foo.html"> <frame src="bar.html"> </frameset> </html> here's what i see on using today's linux commercial bits 2000.06.21.08: <html> <frameset "frameborder="0extra attributes here> <frame src="foo.html"> <frame src="bar.html"> </frameset> </html> methinks this should go over to the Parser group...
Assignee: don → clayton
Component: XP Apps → Parser
QA Contact: sairuh → janc
Let me investigate if it's parser. Over to me. Btw, thanx for the test case.
Assignee: clayton → harishd
This is a very low priority bug. Moving to FUTURE list.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Harish -- First, I agree with your FUTURE approach to this. Second, if we wanted to fix it, we could use code I added a while back to capture ws between attributes so that viewsource would render correctly. I suspect we can use the same mechanism to capture the (invalid) second ".
Adding to bug 57724, tracker for view-source displaying text incorrectly. On Win98 2000121608, I see <frameset frameborder="0" extra attributes here> Reported earlier (for Linux): <frameset "frameborder="0extra attributes here> Correct: <frameset "frameborder="0" extra attributes here> Has it changed, or is it different on different platforms?
Blocks: 57724
OS: Linux → All
Hardware: PC → All
Testcase gives me Not Found The requested URL /foo.html was not found on this server. Apache/1.3.12 Server at bugzilla.mozilla.org Port 80
updated qa contact.
QA Contact: janc → bsharma
When viewing the page source, the source does not wrap to the size of the window. Could someone look into making the lines wrap to the window. This makes reading the source easier than it is right now. Also, is it possible to specify which editor should be used to read the source (though this may not be platform independent)
+dataloss, though it's a borderline case. Jesse: Today's linux is broken in the way win98 was for you, 6 months ago. Ben: ignore that, the issue is with view source, yes the frame targets are 404s. Manoj: this isn't the place for that request. That should be a new bug with serverity=enhancement, if there isn't one already (pretty sure there is). All: should this depend on 55583 or the other one that's regarding retrieving original non-parsed source for things like 'save'?
Keywords: dataloss
jmd: Fixing bug 55583 wouldn't fix all of the view source munging problems, because view source would still go through the parser to become colored.
Summary: view page source not displaying correctly → view source displays attributes with quotes in strange places incorrectly
non-colored view source is still being run through the parser? this bug still shows with syntax coloring off...
No longer blocks: 57724
*** This bug has been marked as a duplicate of 57724 ***
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → DUPLICATE
QA Contact: bsharma → moied
Reopening 57724 dependencies for independent resolution.
Status: RESOLVED → REOPENED
Depends on: 57724
Resolution: DUPLICATE → ---
Works fine on today's Linux CVS with and without syntax highlighting. Resolving WORKSFORME.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → WORKSFORME
I still see this. My testcase line should read: <frameset "frameborder="0" extra attributes here> ^ view source isn't showing the first quote. Reopening on choess' request.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
I believe I'm experiencing this same bug. My HTML code contains: ================================================ <form action="foo.cgi"> <input type="hidden" name="tagline" value="begin \"double\" \'single\' <inside> \\slash\\ end">; <input type="submit" value="Save"> </form> ================================================ ...but the "view source" display shows: ++++++++++++++++++++++++++++++++++++++++++++++++ <form action="openlist.phtml"> <input type="hidden" name="tagline" value="\"double\ \'single\' ><inside> \\slash\\ xxx">; <input type="submit" value="Save"> </form> +++++++++++++++++++++++++++++++++++++++++++++++ The second quote mark has been removed. Also please note the extra > that's been added. Disclaimer: I'm not completely sure that the above is valid HTML code. I think this is the way to escape single quotes, double quotes, backslashes, and brackets but it's possible that this isn't valid and the bug is on *my* end.
*** Bug 179569 has been marked as a duplicate of this bug. ***
The correct way to escape a quote is &quot;, not \". But this is still a problem.
*** Bug 184816 has been marked as a duplicate of this bug. ***
By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and <http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss bugs are of critical or possibly higher severity. Only changing open bugs to minimize unnecessary spam. Keywords to trigger this would be crash, topcrash, topcrash+, zt4newcrash, dataloss.
Severity: normal → critical
Darned if I know how "dataloss" got stuck on this. Re-adjusting.
Severity: critical → minor
Keywords: dataloss, mozilla1.1
It's dataloss because users and developers should rightfully assume pasting text from view source is the same as saving that portion of the file. If a "view" function is *changing* data, data may be lost.
We have about a gajilion of these little bugs (mostly deps of bug 57724; I just created bug 188609 for the root cause of this mess). With all due respect, I don't think they meet the criterion of *critical* data loss, which (AFAIK) is meant for stuff like wiping out your form data and doubling your credit card order, etc. I agree it's a problem, but realistically only fixing bug 188609 is going to solve this stuff.
*** Bug 92196 has been marked as a duplicate of this bug. ***
*** Bug 182215 has been marked as a duplicate of this bug. ***
Attached patch patch v1 — — Splinter Review
Some really fun stuff here... This makes us not lose misplaced quotes in attributes. There are issues with newline counting after invalid attributes, but that's another bug altogether. I've tried to keep the behavior of the tokenizer the same in non-view-source mode. The only real change is that in ConsumeQuotedString, I no longer call SkipOver(). In non-view-source mode, subsequent quotes will be handled by the SkipOver call in CAttributeToken::Consume() (when we try to consume the next attribute).
Assignee: harishd → mrbkap
Status: REOPENED → ASSIGNED
Comment on attachment 159332 [details] [diff] [review] patch v1 bz, could you please give this r=? I'm holding off on asking for sr= until I have r=.
Attachment #159332 - Flags: review?(bzbarsky)
*** Bug 220386 has been marked as a duplicate of this bug. ***
Comment on attachment 159332 [details] [diff] [review] patch v1 Yeah, this looks reasonable. I'd like to see some testcases attached to this bug (and maybe checked in as regression tests if we have such for parser?)
Attachment #159332 - Flags: review?(bzbarsky) → review+
Comment on attachment 159332 [details] [diff] [review] patch v1 rbs, would you sr?
Attachment #159332 - Flags: superreview?(rbs)
Attached file more testcases —
This should cover most of the cases. One thing this uncovers is that though we don't allow " to be in attribute names, we do allow ' to be. Is this desired? There does seem to be a parser regression test suite, but I can't get it to run.
There are subtle differences, goto line 9:45 on the Source-of, or the 'a='b'' becomes 'a="b" '="" in the DOM-Source. But I don't mind either way unless other people have objections. Regarding the patch, why do you GetChar() instead of SkipOver(). Looks like you copy the same char (quote) over itself, no? What specific problem does it fix?
(In reply to comment #35) > There are subtle differences, goto line 9:45 on the Source-of, or the 'a='b'' > becomes 'a="b" '="" in the DOM-Source. But I don't mind either way unless other > people have objections. Once I fix bug 261503 this is going to turn into the same thing as when we have extra double-quotes (technically, it should turn into ="" in the DOM source and when we parse HTML for the real content model as opposed to view-source, those extraneous quotes should be removed). Once that's fixed, my eventual plan is to nuke the attributes that generate the source |=""| in CNavDTD (after getting their newline count) since this makes us assert (since these attributes have no name). > > Regarding the patch, why do you GetChar() instead of SkipOver(). Looks like you > copy the same char (quote) over itself, no? What specific problem does it fix? If you're referring to ConsumeQuotedString, the reason is that if we have: <a href="b""> then that call to SkipOver loses the second quote, so view-source misses it. I need to actually GetChar() on it because ConsumeUntil doesn't actually consume the end condition. bz, do you have any problems with the new parsing?
No, I think the new parsing of such totally bogus stuff is OK (and will get better once bug 261503 is fixed).
For additional context... My symptom for this problem was in WYSIWYG Pro (a JS-based HTML editor). I was trying to coerce it into service as a template editor, but when I put certain Smarty tags inside an attribute the editor was destructive to the content that I was editing, causing Smarty to fail. I eventually determined that it was Mozilla (gecko?) whose parser was getting confused. It's my theory that if the parser did a better job of ignoring what it didn't understand, I might succeed in my original goal.
Comment on attachment 159332 [details] [diff] [review] patch v1 sr=rbs I now see why you needed to turn SkipOver into GetChar. The skip _loops_ until it encounter a different char... Patch looks fine. Not much to expect from sloppy markups.
Attachment #159332 - Flags: superreview?(rbs) → superreview+
checked in
Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 20 years ago
Resolution: --- → FIXED
*** Bug 262562 has been marked as a duplicate of this bug. ***
Can this fix be landed on aviary?
No. This is very much an alpha change.
*** Bug 213299 has been marked as a duplicate of this bug. ***
*** Bug 265638 has been marked as a duplicate of this bug. ***
*** Bug 270792 has been marked as a duplicate of this bug. ***
*** Bug 271217 has been marked as a duplicate of this bug. ***
Looking at the attachment "more testcases" with Firefox 1.0 (Gentoo Linux), I have the same problems again. None of the badly quoted attributes are displayed correctly in the view source window. That's very bad for webdesingers debugging there dynamicly generated HTML code. Can anybody confirm these problems with the latest Firefox release?
Daniel, Firefox 1.0 is using the Gecko engine from last April. None of these fixes are in Firefox 1.0. They will be in Firefox 1.1.
*** Bug 276972 has been marked as a duplicate of this bug. ***
verified with mozilla 1.8a6 2005-01-11-05-trunk
Status: RESOLVED → VERIFIED
*** Bug 285888 has been marked as a duplicate of this bug. ***
*** Bug 286109 has been marked as a duplicate of this bug. ***
*** Bug 292253 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: