Bug 215677 - Move node geometry functions from Range to RenderObject
Summary: Move node geometry functions from Range to RenderObject
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-19 18:13 PDT by Darin Adler
Modified: 2020-08-24 10:13 PDT (History)
22 users (show)

See Also:

Attachments
Patch (43.60 KB, patch)
2020-08-19 21:05 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (47.03 KB, patch)
2020-08-20 09:57 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (50.33 KB, patch)
2020-08-20 13:27 PDT, Darin Adler 		zalan: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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).