Bug 177265 – Spelling and grammar dots should not overlap

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Daniel Bates]

Spelling and grammar error dots may overlap. But they should not.

To see this issue you can perform the following steps in Safari.

1. Ensure that Check Spelling While Typing and Check Grammar with Spelling are enabled under Edit > Spelling and Grammar.
2. Visit <data:text/html,<div contenteditable="true"></div><script>document.querySelector("div").focus();document.execCommand("InsertText", false, "to mooof or not to mooof. ");document.querySelector("div").blur();</script>>

Compare the visual appearance in Safari with the visual appearance of copying and pasting "to mooof or not to mooof. " in TextEdit.

[Radar WebKit Bug Importer]

<rdar://problem/34556424>

[Daniel Bates]

Created attachment 321366 [details]
Patch and unit tests

The implementation of subdivide() is derived from an algorithm that Said Abou-Hallawa came up with. I added unit tests to avoid creating platform-specific pixel tests (we do not run pixel tests by default).

[Build Bot]

Attachment 321366 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/Subrange.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Daniel Bates]

Created attachment 321368 [details]
[Screenshot] Before patch

The screenshot reflects the behavior in shipping Safari as of the time of writing (dots are painted upside down; corrected in bug #176949).

[Daniel Bates]

Created attachment 321369 [details]
[Screenshot] After patch

The screenshot reflects a WebKit build that includes the fix for bug #176949.

[Daniel Bates]

Created attachment 321370 [details]
[Screenshot] TextEdit

[Daniel Bates]

Created attachment 321371 [details]
Patch and unit tests

[Build Bot]

Attachment 321371 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/Subrange.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Daniel Bates]

Created attachment 321373 [details]
Patch and unit tests

[Build Bot]

Attachment 321373 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/Subrange.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 321373 [details]
Patch and unit tests

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

> Source/WebCore/rendering/Subrange.h:34
> +struct Subrange {

I think this name of this class and the file need a little more qualification. Maybe MarkerSubrange?

[Dave Hyatt]

Comment on attachment 321373 [details]
Patch and unit tests

I think TextSubrange may be a little less generic, and lets people know we're talking about text ranges.

[Daniel Bates]

(In reply to Simon Fraser (smfr) from comment #11)
> Comment on attachment 321373 [details]
> Patch and unit tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=321373&action=review
> 
> > Source/WebCore/rendering/Subrange.h:34
> > +struct Subrange {
> 
> I think this name of this class and the file need a little more
> qualification. Maybe MarkerSubrange?

(In reply to Dave Hyatt from comment #12)
> Comment on attachment 321373 [details]
> Patch and unit tests
> 
> I think TextSubrange may be a little less generic, and lets people know
> we're talking about text ranges.

After talking with Dave, I'll take Simon's suggestion and rename the class from Subrange to MarkerSubrange before landing.

[Daniel Bates]

Committed r222298: <http://trac.webkit.org/changeset/222298>