Move more from live range to SimpleRange: callers of absoluteTextRects
Created attachment 396164 [details] Patch
Comment on attachment 396164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396164&action=review > Source/WebCore/dom/Range.cpp:-1183 > -void Range::absoluteTextRects(Vector<IntRect>& rects, bool useSelectionHeight, RangeInFixedPosition* inFixed, OptionSet<BoundingRectBehavior> rectOptions) const I would change this to return the rects instead of using an out parameter. > Source/WebCore/html/HTMLTextFormControlElement.cpp:463 > + return { { { *innerText, 0 }, { *innerText, 0 } } }; Maybe specify SimpleRange here to cut down on some of the curly braces. > Source/WebCore/html/HTMLTextFormControlElement.cpp:488 > + return { { { *startNode, start }, { *endNode, end } } }; Maybe specify SimpleRange here to cut down on some of the curly braces. > Source/WebCore/rendering/RenderObject.cpp:1904 > + else if (is<RenderText>(renderer)) { I assume is<RenderText> will return false here if renderer is null? > Source/WebKitLegacy/mac/WebView/WebFrame.mm:1114 > + return range ? [range textRects] : @[]; range.textRects?
Comment on attachment 396164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396164&action=review Looks like build failures are because WebFrame.mm needs to include DOM.h. >> Source/WebCore/dom/Range.cpp:-1183 >> -void Range::absoluteTextRects(Vector<IntRect>& rects, bool useSelectionHeight, RangeInFixedPosition* inFixed, OptionSet<BoundingRectBehavior> rectOptions) const > > I would change this to return the rects instead of using an out parameter. Yes, agreed. I would do that if I was planning to keep this. But I am planning to delete it. I only removed the argument as a prelude to removing the entire function soon. >> Source/WebCore/html/HTMLTextFormControlElement.cpp:463 >> + return { { { *innerText, 0 }, { *innerText, 0 } } }; > > Maybe specify SimpleRange here to cut down on some of the curly braces. I’d be willing to do that. Not sure it’s better. If you are sure, I will do it. It allows me to remove the outer set. >> Source/WebCore/html/HTMLTextFormControlElement.cpp:488 >> + return { { { *startNode, start }, { *endNode, end } } }; > > Maybe specify SimpleRange here to cut down on some of the curly braces. Ditto. >> Source/WebCore/rendering/RenderObject.cpp:1904 >> + else if (is<RenderText>(renderer)) { > > I assume is<RenderText> will return false here if renderer is null? Yes. >> Source/WebKitLegacy/mac/WebView/WebFrame.mm:1114 >> + return range ? [range textRects] : @[]; > > range.textRects? Will do.
Created attachment 396174 [details] Patch
Anders, you didn’t set review+. Did you mean to?
(In reply to Darin Adler from comment #5) > Anders, you didn’t set review+. Did you mean to? I wanted to wait until everything was green!
Committed r259933: <https://trac.webkit.org/changeset/259933>
<rdar://problem/61644860>