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+

Darin Adler
Reported 2020-08-19 18:13:06 PDT
Move node geometry functions from Range to RenderObject
Attachments
Patch (43.60 KB, patch)
2020-08-19 21:05 PDT, Darin Adler
no flags
Patch (47.03 KB, patch)
2020-08-20 09:57 PDT, Darin Adler
no flags
Patch (50.33 KB, patch)
2020-08-20 13:27 PDT, Darin Adler
zalan: review+
Darin Adler
Comment 1 2020-08-19 21:05:11 PDT Comment hidden (obsolete)
Simon Fraser (smfr)
Comment 2 2020-08-19 21:11:13 PDT
Are you going to hold off from landing this?
Darin Adler
Comment 3 2020-08-19 21:14:08 PDT
Yes.
Darin Adler
Comment 4 2020-08-20 09:57:02 PDT Comment hidden (obsolete)
Darin Adler
Comment 5 2020-08-20 13:27:47 PDT
Darin Adler
Comment 6 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.
alan
Comment 7 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.
Darin Adler
Comment 8 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.
Darin Adler
Comment 9 2020-08-22 09:16:11 PDT
Radar WebKit Bug Importer
Comment 10 2020-08-22 09:17:14 PDT
Aakash Jain
Comment 11 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).
Note You need to log in before you can comment on or make changes to this bug.