Bug 215677

Summary: Move node geometry functions from Range to RenderObject
Product: WebKit Reporter: Darin Adler <darin>
Component: Layout and RenderingAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, aboxhall, apinheiro, bfulgham, cdumez, cfleizach, changseok, dmazzoni, esprehn+autocc, ews-watchlist, glenn, jcraig, jdiggs, kangil.han, kondapallykalyan, mifenton, pdr, samuel_white, sam, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=215551
https://bugs.webkit.org/show_bug.cgi?id=215772
Attachments:
Description Flags
Patch
none
Patch
none
Patch zalan: review+

Description Darin Adler 2020-08-19 18:13:06 PDT 
Move node geometry functions from Range to RenderObject
Comment 1 Darin Adler 2020-08-19 21:05:11 PDT Comment hidden (obsolete)
Comment 2 Simon Fraser (smfr) 2020-08-19 21:11:13 PDT 
Are you going to hold off from landing this?
Comment 3 Darin Adler 2020-08-19 21:14:08 PDT 
Yes.
Comment 4 Darin Adler 2020-08-20 09:57:02 PDT Comment hidden (obsolete)
Comment 5 Darin Adler 2020-08-20 13:27:47 PDT 
Created attachment 406962 [details]
Patch
Comment 6 Darin Adler 2020-08-20 17:08:45 PDT 
Hooray, all tests passing now, so ready for review. I won’t be landing this until Saturday at the soonest, but would love to get review soon.
Comment 7 zalan 2020-08-21 15:40:58 PDT 
Comment on attachment 406962 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406962&action=review

> Source/WebCore/rendering/RenderObject.h:573
> +    WEBCORE_EXPORT static Vector<FloatQuad> absoluteTextQuads(const SimpleRange&, OptionSet<BoundingRectBehavior> = { });
> +    WEBCORE_EXPORT static Vector<IntRect> absoluteTextRects(const SimpleRange&, OptionSet<BoundingRectBehavior> = { });
> +    WEBCORE_EXPORT static Vector<FloatRect> absoluteBorderAndTextRects(const SimpleRange&, OptionSet<BoundingRectBehavior> = { });
> +    static Vector<FloatRect> clientBorderAndTextRects(const SimpleRange&);

It's a bit unfortunate that functions with _text_ and _border_  in their names end up in RenderObject (where we have neither border nor text) but I understand that it's hard to find a better place for a concept (Range) that has no associated renderer.
Comment 8 Darin Adler 2020-08-21 15:46:17 PDT 
Comment on attachment 406962 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406962&action=review

>> Source/WebCore/rendering/RenderObject.h:573
>> +    static Vector<FloatRect> clientBorderAndTextRects(const SimpleRange&);
> 
> It's a bit unfortunate that functions with _text_ and _border_  in their names end up in RenderObject (where we have neither border nor text) but I understand that it's hard to find a better place for a concept (Range) that has no associated renderer.

I am totally willing to follow up and move them somewhere else in the rendering world. They don’t have to stay on RenderObject.
Comment 9 Darin Adler 2020-08-22 09:16:11 PDT 
Committed r266028: <https://trac.webkit.org/changeset/266028>
Comment 10 Radar WebKit Bug Importer 2020-08-22 09:17:14 PDT 
<rdar://problem/67617601>
Comment 11 Aakash Jain 2020-08-24 10:13:26 PDT 
(In reply to Darin Adler from comment #9)
> Committed r266028: <https://trac.webkit.org/changeset/266028>
Seems like it broke one test on ios-wk2: platform/ios/ios/fast/coordinates/range-client-rects.html. Tracked in Bug 215772.

Also, unfortunately ios-wk2 ews was broken for some time late last week (due to Bug 215742), due to which the green ios-wk2 bubble for this bug is misleading (iOS layout-tests weren't actually run).