Following up on bug 112681, we should wire up the newly created event so that it actually fires off on the Document when violations occur.
Created attachment 194003 [details] Patch
Comment on attachment 194003 [details] Patch Attachment 194003 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17191579
Comment on attachment 194003 [details] Patch Attachment 194003 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17178585
Created attachment 194005 [details] Patch
Bots are happy. Would you mind taking a look, Adam?
Comment on attachment 194005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194005&action=review > Source/WebCore/page/ContentSecurityPolicy.cpp:1688 > + if (stack) { prefer early return > Source/WebCore/page/ContentSecurityPolicy.cpp:1692 > + KURL source = KURL(KURL(), callFrame.sourceURL()); This should probably use the ParsedURLString version of the constructor. > Source/WebCore/page/ContentSecurityPolicy.cpp:1718 > + document->enqueueDocumentEvent(SecurityPolicyViolationEvent::create(eventNames().securitypolicyviolationEvent, init)); Do, we always fire the event at the document? I guess that's ok. It seems like it would be better to fire the event at the element that caused the violation, but I guess there isn't always an element...
Thanks, Adam! (In reply to comment #6) > (From update of attachment 194005 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=194005&action=review > > > Source/WebCore/page/ContentSecurityPolicy.cpp:1688 > > + if (stack) { > > prefer early return Good call. I'll fix this and CQ the patch in the morning. > > Source/WebCore/page/ContentSecurityPolicy.cpp:1692 > > + KURL source = KURL(KURL(), callFrame.sourceURL()); > > This should probably use the ParsedURLString version of the constructor. I'll fix this in a separate patch, as I can change the 'report-uri' version I copy/pasted at the same time. > > Source/WebCore/page/ContentSecurityPolicy.cpp:1718 > > + document->enqueueDocumentEvent(SecurityPolicyViolationEvent::create(eventNames().securitypolicyviolationEvent, init)); > > Do, we always fire the event at the document? I guess that's ok. It seems like it would be better to fire the event at the element that caused the violation, but I guess there isn't always an element... As a first pass, I think targeting the document makes sense. It has the advantage of capturing all events related to a page, regardless of whether they occur on attached elements or XHR or whatever. When it's relevant, though, I do think it'd be a good idea to either target the elements directly, or include a reference to the element in the event. *shrug* It's a good discussion to have on the list. :)
Created attachment 194328 [details] Patch for landing
Comment on attachment 194328 [details] Patch for landing Clearing flags on attachment: 194328 Committed r146520: <http://trac.webkit.org/changeset/146520>
All reviewed patches have been landed. Closing bug.
(In reply to comment #7) > > > Source/WebCore/page/ContentSecurityPolicy.cpp:1692 > > > + KURL source = KURL(KURL(), callFrame.sourceURL()); > > > > This should probably use the ParsedURLString version of the constructor. > > I'll fix this in a separate patch, as I can change the 'report-uri' version I copy/pasted at the same time. Split out into bug 112965; thanks again for pointing it out.