Bug 15961 - View source mode is missing the processing instruction line.
Summary: View source mode is missing the processing instruction line.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Elliott Sprehn
URL:
Keywords: HasReduction, InRadar
Depends on:
Blocks:
 
Reported: 2007-11-12 16:11 PST by Anyang Ren
Modified: 2013-01-02 12:49 PST (History)
4 users (show)

See Also:


Attachments
Preliminary patch for discussion (2.04 KB, patch)
2007-11-12 16:31 PST, Anyang Ren
no flags Details | Formatted Diff | Diff
test case (185 bytes, text/html)
2008-01-14 15:38 PST, Eric Seidel (no email)
no flags Details
Patch (1.84 KB, patch)
2013-01-02 12:27 PST, Elliott Sprehn
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anyang Ren 2007-11-12 16:11:01 PST
In the view source mode, the processing instruction line is missing in the output.

For example, the first line of the HTML page at http://alioth.debian.org/ is
a processing instruction:
<?xml version="1.0" encoding="UTF-8"?>

This line is missing in the view source mode output.  I tracked it down to the
HTMLTokenizer class, specifically the HTMLTokenizer::write and
HTMLTokenizer::parseProcessingInstruction methods.
Comment 1 Anyang Ren 2007-11-12 16:31:39 PST
Created attachment 17217 [details]
Preliminary patch for discussion

This patch is intended to show where the problem is, even though
it is in the form of a fix.  I hope that the WebKit team can come up
with a better fix.

In HTMLTokenizer::write, after the call to parseProcessingInstruction
returns, we continue the while loop.  So we skip the processToken call
at the end of the switch statement, and the line is not added to the
HTMLViewSourceDocument by the addViewSourceToken call in
HTMLTokenizer::processToken.

I first tried removing the "continue" statement so that we fall through to
the processToken call after the switch statement.  But the parseTag call
after the processToken call caused an empty "<>" tag to be added after
the processing instruction line.  So I'm now calling processToken before
the continue statement.

The other changes in this patch add the characters of the processing
instruction line to 'buffer' (via the 'dest' pointer) so that they show up
in the output.  It is also necessary to remove the state.setDiscardLF(true)
statement so that we don't lose the LF after the processing instruction line
in the output.

Note that I mark the processing instruction line as a textAtom simply because
it is easier to work with HTMLTokenizer::processToken and
HTMLViewSourceDocument::addViewSourceToken to output a textAtom.
I believe the processing instruction line should be handled as a tag.
Comment 2 Anyang Ren 2007-11-13 13:28:47 PST
David, I'd appreciate your comments on this view source mode
bug and the right way to fix it.  Thanks.
Comment 3 David Kilzer (:ddkilzer) 2007-11-13 20:32:23 PST
Viewing the source of http://alioth.debian.org/ from Safari 3 Public Beta with a local debug build of WebKit r27716 on Tiger shows the <?xml...> tag for me.

Which Safari/WebKit are you seeing this on?

Please set the "review?" flag if you want the patch reviewed for comments.

Comment 4 Anyang Ren 2007-11-14 11:42:09 PST
Comment on attachment 17217 [details]
Preliminary patch for discussion

I'm using WebKit/WebCore/html/HTMLTokenizer.cpp, r25754.
I'm not using Safari.  Safari's "View Source" command doesn't seem to be
implemented using the "view source mode" of the HTMLTokenizer class.
Comment 5 Anyang Ren 2007-11-14 14:40:51 PST
(In reply to comment #3)
> Viewing the source of http://alioth.debian.org/ from Safari 3 Public Beta with
> a local debug build of WebKit r27716 on Tiger shows the <?xml...> tag for me.

You can confirm this bug using the Source view in the Web Inspector.

Comment 6 Maciej Stachowiak 2007-11-22 23:00:45 PST
Comment on attachment 17217 [details]
Preliminary patch for discussion

I'm not 100% sure if this change is right, it seems generally ok.

However, it needs a ChangeLog and a layout test. You can force view source mode by putting the relevant contents into an <iframe> element with the viewsource attribute set.

r- for lack of test case and ChangeLog.
Comment 7 Eric Seidel (no email) 2008-01-14 15:38:28 PST
Created attachment 18449 [details]
test case

Here is a test case which I believe demonstrates the bug.
Comment 8 Adam Roben (:aroben) 2008-01-29 11:04:07 PST
<rdar://problem/5712854>
Comment 9 Elliott Sprehn 2012-08-15 00:38:53 PDT
This is actually fixed, probably by the HTML5 parser rewrite. We should just add a layout test and close this bug.

(Note the patch here is impossible to apply as well since HTMLTokenizer has been entirely rewritten.)
Comment 10 Elliott Sprehn 2013-01-02 12:27:03 PST
Created attachment 181046 [details]
Patch
Comment 11 WebKit Review Bot 2013-01-02 12:49:50 PST
Comment on attachment 181046 [details]
Patch

Clearing flags on attachment: 181046

Committed r138637: <http://trac.webkit.org/changeset/138637>
Comment 12 WebKit Review Bot 2013-01-02 12:49:54 PST
All reviewed patches have been landed.  Closing bug.