Bug 154127 – Soft spaces are often tracked in the wrong spot

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Beth Dakin]

Soft spaces are often tracked in the wrong spot

rdar://problem/24493140

[Beth Dakin]

Created attachment 271086 [details]
Patch

[WebKit Commit Bot]

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.

[Beth Dakin]

Created attachment 271088 [details]
Patch

[WebKit Commit Bot]

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.

[Sam Weinig]

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

[Beth Dakin]

Thanks Sam! http://trac.webkit.org/changeset/196452

[Alexey Proskuryakov]

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?

[Beth Dakin]

(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.

[Beth Dakin]

(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.