Closed Bug 912981 Opened 10 years ago Closed 10 years ago

doctype error incorrectly flagged for srcdoc URLs


(Core :: DOM: HTML Parser, defect)

Windows 8
Not set



Tracking Status
firefox24 --- unaffected
firefox25 --- affected
firefox26 --- affected


(Reporter: jkitch, Assigned: jkitch)



(3 files, 2 obsolete files)

Attached file test.html
Load the attached file, open the context menu for the iframe, select "This Frame" and "View Frame Source"

Actual result:

The <p> tag is highlighted in red because a missing doctype error is reported.

Expected result:

No error should be reported.
The doctype element is optional within srcdoc iframes.
Assignee: nobody → jkitch.bug
Component: View Source → HTML: Parser
Product: Toolkit → Core
It turns out users of nsIViewSourceChannels need to be aware of the srcdoc status of the embedded channel.

I'm not extending knowledge of this status to nsIDocument because I haven't discovered a situation where it matters.  It may also cause unintended side-effects.
Attachment #800751 - Flags: review?(bzbarsky)
Attached patch Part 2: parser changes (obsolete) — Splinter Review
Need to check nsIViewSourceChannel as well as nsIInputStreamChannel.

I have tested this locally, but does this need an automated test?  If so, could you provide some guidance how to perform the test or alternatively point out other tests which assess parser error reporting?
Attachment #800753 - Flags: review?(hsivonen)
QA Contact: alexandra.lucinet
Comment on attachment 800751 [details] [diff] [review]
Part 1 View-Source Channel changes

>+   *aIsSrcdocChannel = mIsSrcdocChannel;

Please fix the indent?

Attachment #800751 - Flags: review?(bzbarsky) → review+
For part 2, seems like we may want an "IsSrcdocChannel" helper in netutil or something..
The isSrcdocChannel check now resides within nsNetUtil.h
Attachment #800751 - Attachment is obsolete: true
Attachment #801219 - Flags: review?(bzbarsky)
This moves the test for srcdoc status out of the parser.

The question of testing (see comment 2) still applies.
Attachment #800753 - Attachment is obsolete: true
Attachment #800753 - Flags: review?(hsivonen)
Attachment #801220 - Flags: review?(hsivonen)
Attachment #801220 - Flags: review?(hsivonen) → review+
Comment on attachment 801219 [details] [diff] [review]
part 1: netwerk changes

Attachment #801219 - Flags: review?(bzbarsky) → review+
Attachment #801220 - Attachment description: parser → part 2: parser changes
Keywords: checkin-needed
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Comment on attachment 801219 [details] [diff] [review]
part 1: netwerk changes

Review of attachment 801219 [details] [diff] [review]:

::: netwerk/base/public/nsNetUtil.h
@@ +2345,5 @@
> +{
> +  bool isSrcdoc;
> +  nsCOMPtr<nsIInputStreamChannel> isr = do_QueryInterface(aChannel);
> +  if (isr) {
> +    isr->GetIsSrcdocChannel(&isSrcdoc);

This looks unsafe in general... I don't see a reason to assume that this initializes isSrcdoc.
Could you explain your reasoning?  I cannot see the problem.

If the channel is neither an nsIInputStreamChannel or a nsIViewSourceChannel, isSrcdoc isn't used and a constant false is returned.

If you mean that the value returned by GetIsSrcdocChannel(bool *aIsSrcdocChannel), I believe that it is initialised on object creation.

>     nsInputStreamChannel() :
>       mIsSrcdocChannel(false) {}

Are there other ways that an nsInputStreamChannel can be created?
Flags: needinfo?(Ms2ger)
GetIsSrcdocChannel is an xpcom method, so it is not required to actually set its outparam if it returns a failure nsresult. Since you don't check the result, you can't be sure that that hasn't happened.
Flags: needinfo?(Ms2ger)
You need to log in before you can comment on or make changes to this bug.