Summary: | document.origin doesn't match spec | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Boris Zbarsky <bzbarsky> | ||||||||
Component: | DOM | Assignee: | Chris Dumez <cdumez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, ap, buildbot, cdumez, commit-queue, dbates, esprehn+autocc, kangil.han, mcatanzaro, mike, mkwst, rniwa | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=17352 https://bugs.webkit.org/show_bug.cgi?id=115643 |
||||||||||
Attachments: |
|
Description
Boris Zbarsky
2017-02-08 15:00:45 PST
I can reproduce. Thanks for the bug report. I'll take a look soon. Note that there is a proposal from Blink to just remove document.origin altogether in favor of self.origin. Created attachment 301208 [details]
WIP Patch
Comment on attachment 301208 [details] WIP Patch Attachment 301208 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3065179 New failing tests: http/tests/media/media-stream/enumerate-devices-source-id-persistent.html Created attachment 301214 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 301237 [details]
Patch
(In reply to comment #6) > Chris, I'm not sure whether you saw comment 2? I saw. I don't think we would remove it short-term though. So for now, it is better to match the spec and other shipping browsers. If we end up removing it later on (after we implement self.origin), so be it. Comment on attachment 301237 [details] Patch Clearing flags on attachment: 301237 Committed r212178: <http://trac.webkit.org/changeset/212178> All reviewed patches have been landed. Closing bug. This code was added in https://trac.webkit.org/changeset/164036, and it's really surprising if it wasn't breaking a whole lot of things besides the visible value of document.origin. Is it really not used anywhere else in the engine? +PassRefPtr<Document> Document::create(ScriptExecutionContext& context) +{ + RefPtr<Document> document = adoptRef(new Document(nullptr, URL())); + document->setSecurityOrigin(context.securityOrigin()); + return document; +} + ... +String Document::origin() const +{ + return securityOrigin()->databaseIdentifier(); +} + (In reply to comment #11) > This code was added in https://trac.webkit.org/changeset/164036, and it's > really surprising if it wasn't breaking a whole lot of things besides the > visible value of document.origin. Is it really not used anywhere else in the > engine? > > +PassRefPtr<Document> Document::create(ScriptExecutionContext& context) > +{ > + RefPtr<Document> document = adoptRef(new Document(nullptr, URL())); > + document->setSecurityOrigin(context.securityOrigin()); > + return document; > +} > + > > ... > > +String Document::origin() const > +{ > + return securityOrigin()->databaseIdentifier(); > +} > + What do you mean by anywhere *else*. Document::origin() is not used *anywhere* in the engine and therefore my patch only changes the value exposed to the Web to match the specification and other browsers. Our internal code relies on securityOrigin(), not origin(). As the diff shows, the original change was setting securityOrigin from origin, so both were incorrect. Sorry, misread the original patch. |