Bug 154127 - Soft spaces are often tracked in the wrong spot
Summary: Soft spaces are often tracked in the wrong spot
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-11 13:46 PST by Beth Dakin
Modified: 2016-02-11 16:24 PST (History)
6 users (show)

See Also:


Attachments
Patch (26.96 KB, patch)
2016-02-11 14:18 PST, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch (26.91 KB, patch)
2016-02-11 14:27 PST, Beth Dakin
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2016-02-11 13:46:10 PST
Soft spaces are often tracked in the wrong spot

rdar://problem/24493140
Comment 1 Beth Dakin 2016-02-11 14:18:07 PST
Created attachment 271086 [details]
Patch
Comment 2 WebKit Commit Bot 2016-02-11 14:19:31 PST
Attachment 271086 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebPageProxy.h:564:  The parameter name "editingRangeIsRelativeTo" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/mac/WebView/WebHTMLView.mm:7107:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.h:1139:  The parameter name "editingRangeIsRelativeTo" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/mac/WebView/WebFrame.mm:822:  The parameter name "INT_MAX" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 4 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Beth Dakin 2016-02-11 14:27:02 PST
Created attachment 271088 [details]
Patch
Comment 4 WebKit Commit Bot 2016-02-11 14:28:10 PST
Attachment 271088 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/WebView/WebFrame.mm:822:  The parameter name "INT_MAX" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Sam Weinig 2016-02-11 15:29:26 PST
Comment on attachment 271088 [details]
Patch

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

Please do the refactor we discussed so we don't have the duplicated code.  Also, please file a follow up bug to add the test infrastructure for testing this.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:5819
> +    process().send(Messages::WebPage::InsertTextAsync(text, replacementRange, registerUndoGroup, (uint32_t)editingRangeIsRelativeTo), m_pageID);

static_cast please.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4334
> +        RefPtr<Range> replacementRange = rangeFromEditingRange(frame, replacementEditingRange, (EditingRangeIsRelativeTo)editingRangeIsRelativeTo);

static_cast
Comment 6 Beth Dakin 2016-02-11 15:39:45 PST
Thanks Sam! http://trac.webkit.org/changeset/196452
Comment 7 Alexey Proskuryakov 2016-02-11 16:10:48 PST
Comment on attachment 271088 [details]
Patch

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

> Source/WebKit2/Shared/EditingRange.h:35
> +    Document,

Is it Document, or editable root?
Comment 8 Beth Dakin 2016-02-11 16:13:10 PST
(In reply to comment #7)
> Comment on attachment 271088 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=271088&action=review
> 
> > Source/WebKit2/Shared/EditingRange.h:35
> > +    Document,
> 
> Is it Document, or editable root?

Good point. Editable root is really more accurate. I will update the name.
Comment 9 Beth Dakin 2016-02-11 16:24:39 PST
(In reply to comment #8)
> (In reply to comment #7)
> > Comment on attachment 271088 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=271088&action=review
> > 
> > > Source/WebKit2/Shared/EditingRange.h:35
> > > +    Document,
> > 
> > Is it Document, or editable root?
> 
> Good point. Editable root is really more accurate. I will update the name.

I filed https://bugs.webkit.org/show_bug.cgi?id=154138 and posted a patch to re-name.