Closed
Bug 912981
Opened 10 years ago
Closed 10 years ago
doctype error incorrectly flagged for srcdoc URLs
Categories
(Core :: DOM: HTML Parser, defect)
Tracking
()
RESOLVED
FIXED
mozilla26
Tracking | Status | |
---|---|---|
firefox24 | --- | unaffected |
firefox25 | --- | affected |
firefox26 | --- | affected |
People
(Reporter: jkitch, Assigned: jkitch)
Details
Attachments
(3 files, 2 obsolete files)
50 bytes,
text/html
|
Details | |
5.95 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
7.74 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
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 | ||
Updated•10 years ago
|
Assignee: nobody → jkitch.bug
Component: View Source → HTML: Parser
Product: Toolkit → Core
Assignee | ||
Comment 1•10 years ago
|
||
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)
Assignee | ||
Comment 2•10 years ago
|
||
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)
Assignee | ||
Updated•10 years ago
|
status-firefox24:
--- → unaffected
status-firefox25:
--- → affected
status-firefox26:
--- → affected
Updated•10 years ago
|
QA Contact: alexandra.lucinet
![]() |
||
Comment 3•10 years ago
|
||
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+
![]() |
||
Comment 4•10 years ago
|
||
For part 2, seems like we may want an "IsSrcdocChannel" helper in netutil or something..
Assignee | ||
Comment 5•10 years ago
|
||
The isSrcdocChannel check now resides within nsNetUtil.h
Attachment #800751 -
Attachment is obsolete: true
Attachment #801219 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 6•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #801220 -
Flags: review?(hsivonen) → review+
![]() |
||
Comment 7•10 years ago
|
||
Comment on attachment 801219 [details] [diff] [review] part 1: netwerk changes r=me
Attachment #801219 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #801220 -
Attachment description: parser → part 2: parser changes
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e473e727e96 https://hg.mozilla.org/integration/mozilla-inbound/rev/e77fba8de25f
Keywords: checkin-needed
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9e473e727e96 https://hg.mozilla.org/mozilla-central/rev/e77fba8de25f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Comment 10•10 years ago
|
||
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.
Assignee | ||
Comment 11•10 years ago
|
||
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)
Comment 12•10 years ago
|
||
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.
Description
•