Bug 171393

Summary: Range.getClientRects() / getBoundingClientRect() should return DOMRect types
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: CSSAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, buildbot, cmarcelo, darin, dbates, esprehn+autocc, fred.wang, ggaren, kangil.han, kondapallykalyan, sam, simon.fraser
Priority: P2 Keywords: WebExposed
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://drafts.csswg.org/cssom-view/#extensions-to-the-range-interface
See Also: https://bugs.webkit.org/show_bug.cgi?id=171226
Bug Depends on:    
Bug Blocks: 171412, 171418    
Attachments:
Description Flags
WIP Patch
none
Patch
none
Patch
none
Patch simon.fraser: review+, simon.fraser: commit-queue-

Chris Dumez
Reported 2017-04-27 14:25:42 PDT
Range.getClientRects() / getBoundingClientRect() should return DOMRect types: - https://drafts.csswg.org/cssom-view/#extensions-to-the-range-interface
Attachments
WIP Patch (5.06 KB, patch)
2017-04-27 16:43 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (9.25 KB, patch)
2017-04-27 16:54 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (14.67 KB, patch)
2017-04-28 07:39 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (14.80 KB, patch)
2017-04-28 08:25 PDT, Chris Dumez 		simon.fraser: review+
simon.fraser: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2017-04-27 16:43:19 PDT
Created attachment 308476 [details] WIP Patch
Chris Dumez
Comment 2 2017-04-27 16:54:46 PDT
Created attachment 308478 [details] Patch
Sam Weinig
Comment 3 2017-04-28 06:11:32 PDT
Comment on attachment 308478 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308478&action=review > Source/WebCore/dom/DOMRect.h:44 > + static Vector<Ref<DOMRect>> createVector(const Vector<FloatQuad>&); I think its a little weird to have a "create" function, even one with a suffix, as a class function if it doesn't return the class (or a Ref<> of said class). I think this would be clearer as a free function. > Source/WebCore/dom/Range.h:-38 > -class ClientRectList; Are there any remaining uses of ClientRect or ClientRectList? Can we remove them?
Chris Dumez
Comment 4 2017-04-28 06:41:55 PDT
(In reply to Sam Weinig from comment #3) > Comment on attachment 308478 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=308478&action=review > > > Source/WebCore/dom/DOMRect.h:44 > > + static Vector<Ref<DOMRect>> createVector(const Vector<FloatQuad>&); > > I think its a little weird to have a "create" function, even one with a > suffix, as a class function if it doesn't return the class (or a Ref<> of > said class). I think this would be clearer as a free function. Ok, will do. > > > Source/WebCore/dom/Range.h:-38 > > -class ClientRectList; > > Are there any remaining uses of ClientRect or ClientRectList? Can we remove > them? Yes and yes. See related radars. I am doing this gradually.
Chris Dumez
Comment 5 2017-04-28 07:39:25 PDT
Created attachment 308529 [details] Patch
Chris Dumez
Comment 6 2017-04-28 08:25:56 PDT
Created attachment 308530 [details] Patch
Chris Dumez
Comment 7 2017-04-28 08:35:12 PDT
(In reply to Chris Dumez from comment #4) > (In reply to Sam Weinig from comment #3) > > Comment on attachment 308478 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=308478&action=review > > > > > Source/WebCore/dom/DOMRect.h:44 > > > + static Vector<Ref<DOMRect>> createVector(const Vector<FloatQuad>&); > > > > I think its a little weird to have a "create" function, even one with a > > suffix, as a class function if it doesn't return the class (or a Ref<> of > > said class). I think this would be clearer as a free function. > > Ok, will do. > > > > > > Source/WebCore/dom/Range.h:-38 > > > -class ClientRectList; > > > > Are there any remaining uses of ClientRect or ClientRectList? Can we remove > > them? > > Yes and yes. See related radars. I am doing this gradually. I meant related bugs: https://bugs.webkit.org/show_bug.cgi?id=171412 https://bugs.webkit.org/show_bug.cgi?id=171393
Simon Fraser (smfr)
Comment 8 2017-04-28 14:14:36 PDT
Comment on attachment 308530 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308530&action=review > Source/WebCore/WebCore.xcodeproj/project.pbxproj:3325 > 83120C711C56F3FB001CB112 /* HTMLDataElement.h in Headers */ = {isa = PBXBuildFile; fileRef = 834B86A71C56E83A00F3F0E3 /* HTMLDataElement.h */; }; > + 83149FF61EB38B3700089665 /* DOMRect.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83149FF51EB38B1200089665 /* DOMRect.cpp */; }; Should probably run sort-Xcode-project-file
Chris Dumez
Comment 9 2017-04-28 14:16:35 PDT
Committed r215946: <http://trac.webkit.org/changeset/215946>
Note You need to log in before you can comment on or make changes to this bug.