Bug 171846

Summary: Drop custom bindings code for Window.location setter
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cdumez, commit-queue, darin, esprehn+autocc, ggaren, kondapallykalyan, rniwa, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://html.spec.whatwg.org/#the-window-object
See Also: https://bugs.webkit.org/show_bug.cgi?id=171843
Attachments:
Description Flags
Patch none

Chris Dumez
Reported 2017-05-08 21:22:18 PDT
Drop custom code for Window.location setter. The custom code was needed to fix a bug that was fixed a long time ago (rdar://problem/5695330).
Attachments
Patch (3.91 KB, patch)
2017-05-09 10:48 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2017-05-08 22:07:42 PDT
Grumph. PutForwards currently does not work for window.location because DOMWindow::location() can currently return null and our bindings generator does not handle that. We could add support in the bindings generator but I believe DOMWindow::location() is never supposed to return null. WebKit currently associates Location with the frame instead of the Window, which is not as per spec (and does not match Firefox or Chrome).
Chris Dumez
Comment 2 2017-05-09 10:48:53 PDT
Created attachment 309514 [details] Patch
Sam Weinig
Comment 3 2017-05-09 12:48:18 PDT
Comment on attachment 309514 [details] Patch v.nice
Sam Weinig
Comment 4 2017-05-09 12:50:23 PDT
(In reply to Chris Dumez from comment #1) > Grumph. PutForwards currently does not work for window.location because > DOMWindow::location() can currently return null and our bindings generator > does not handle that. > > We could add support in the bindings generator but I believe > DOMWindow::location() is never supposed to return null. WebKit currently > associates Location with the frame instead of the Window, which is not as > per spec (and does not match Firefox or Chrome). We really need to fix up our window/document/frame mess. It's long past due. (Do we rename Frame to BrowsingContext like madmen?)
Chris Dumez
Comment 5 2017-05-09 13:01:10 PDT
Comment on attachment 309514 [details] Patch Clearing flags on attachment: 309514 Committed r216534: <http://trac.webkit.org/changeset/216534>
Chris Dumez
Comment 6 2017-05-09 13:01:12 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.