doctype error incorrectly flagged for srcdoc URLs

RESOLVED FIXED in mozilla26

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jkitch, Assigned: jkitch)

Tracking

unspecified
mozilla26
x86_64
Windows 8
Points:
---

Firefox Tracking Flags

(firefox24 unaffected, firefox25 affected, firefox26 affected)

Details

Attachments

(3 attachments, 2 obsolete attachments)

Posted 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)
Posted 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?

r=me
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

r=me
Attachment #801219 - Flags: review?(bzbarsky) → review+
Attachment #801220 - Attachment description: parser → part 2: parser changes
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9e473e727e96
https://hg.mozilla.org/mozilla-central/rev/e77fba8de25f
Status: NEW → RESOLVED
Closed: 6 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.