Bug 135631 - Cleanup InlineTextBox::paintSelection and ::localSelectionRect.
Summary: Cleanup InlineTextBox::paintSelection and ::localSelectionRect.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on: 135657
Blocks:
  Show dependency treegraph
 
Reported: 2014-08-05 18:40 PDT by zalan
Modified: 2017-11-15 13:11 PST (History)
8 users (show)

See Also:


Attachments
Patch (6.87 KB, patch)
2014-08-05 19:29 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (6.79 KB, patch)
2014-08-06 07:25 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2014-08-05 18:40:25 PDT
SSIA.
Comment 1 zalan 2014-08-05 19:29:28 PDT
Created attachment 236074 [details]
Patch
Comment 2 Darin Adler 2014-08-05 20:45:35 PDT
Comment on attachment 236074 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No change in functionality.

But we removed the snapping from localSelectionRect. That seems like a bug fix, not a “no change” refactoring fix.

> Source/WebCore/rendering/InlineTextBox.cpp:740
> +    LayoutUnit selectionTop = root().selectionTopAdjustedForPrecedingBlock();

No need to put this into local variable any more if we are only using this once.
Comment 3 zalan 2014-08-06 07:25:31 PDT
Created attachment 236101 [details]
Patch
Comment 4 WebKit Commit Bot 2014-08-06 08:04:30 PDT
Comment on attachment 236101 [details]
Patch

Clearing flags on attachment: 236101

Committed r172145: <http://trac.webkit.org/changeset/172145>
Comment 5 WebKit Commit Bot 2014-08-06 08:04:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 WebKit Commit Bot 2014-08-06 10:41:14 PDT
Re-opened since this is blocked by bug 135657
Comment 7 Daniel Bates 2017-10-19 12:36:35 PDT
Marked this bug resolved fix following the landing of <https://trac.webkit.org/changeset/223699> (bug #178232) that shared more code between InlineTextBox::paintSelection() and InlineTextBox::localSelectionRect(). The pointer to reference conversions that were made in the proposed patch for this bug (attachment #236101 [details]) were made in <https://trac.webkit.org/changeset/174876> (bug #137858).

For completeness, we cannot remove the logic to compute the enclosing integral rectangle from localSelectionRect() as the patch on this bug (attachment #236101 [details]) attempted to do because localSelectionRect() is used to answer calls to the Web API Element.getClientRects() and other objects that override localSelectionRect() return the enclosing integral rectangle of the selection. For consistency we would need to update all overrides of localSelectionRect(), including the override in InlineTextBox. Bug #138913 is tracking this effort.
Comment 8 Radar WebKit Bug Importer 2017-11-15 13:11:13 PST
<rdar://problem/35568938>