Source/WebCore:
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Sep 2016 21:29:47 +0000 (21:29 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Sep 2016 21:29:47 +0000 (21:29 +0000)
Text replacement candidates don't always overwrite the entire original string
https://bugs.webkit.org/show_bug.cgi?id=161779
<rdar://problem/28033492>

Patch by Tim Horton <timothy_horton@apple.com> on 2016-09-09
Reviewed by Simon Fraser.

New test: editing/mac/spelling/accept-candidate-replacing-multiple-words.html.

* editing/Editor.cpp:
(WebCore::Editor::contextRangeForCandidateRequest):
Factor contextRangeForCandidateRequest out of the WebKits, into Editor.
This just expands to paragraph boundaries from the cursor.

(WebCore::Editor::selectTextCheckingResult):
Add selectTextCheckingResult, which, given a TextCheckingResult,
selects the range represented by the result's location and length, which
indicate the portion of the context string that the result refers to.
In the case of accepting a candidate, we want to select that range
so that our insertion will overwrite it.

(WebCore::Editor::handleAcceptedCandidate):
Make use of selectTextCheckingResult instead of just assuming that we want
to replace the word to the left of the insertion point.

(WebCore::Editor::stringForCandidateRequest): Deleted.
* editing/Editor.h:

* testing/Internals.cpp:
(WebCore::Internals::handleAcceptedCandidate):
* testing/Internals.h:
* testing/Internals.idl:
Internals' handleAcceptedCandidate assumed (wrongly) that the length
of a TextCheckerResult was the length of the candidate, when really it is
the length of the text that the candidate would replace. Adjust this,
and expose the replacement range to JavaScript, so we can test this.

Tools:
URLParser: Fix and optimize parsing file URLs ending with a host but no slash
https://bugs.webkit.org/show_bug.cgi?id=161815

Reviewed by Geoffrey Garen.

* TestWebKitAPI/Tests/WebCore/URLParser.cpp:
(TestWebKitAPI::TEST_F):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@205768 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/URLParser.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp

index 8cf3d8e..4b6fd48 100644 (file)
 
 2016-09-09  Alex Christensen  <achristensen@webkit.org>
 
+        URLParser: Fix and optimize parsing file URLs ending with a host but no slash
+        https://bugs.webkit.org/show_bug.cgi?id=161815
+
+        Reviewed by Geoffrey Garen.
+
+        Covered by new API tests.
+
+        * platform/URLParser.cpp:
+        (WebCore::bufferView):
+        (WebCore::URLParser::copyURLPartsUntil):
+        (WebCore::URLParser::parse):
+
+2016-09-09  Alex Christensen  <achristensen@webkit.org>
+
         URLParser: Handle \ in path according to spec
         https://bugs.webkit.org/show_bug.cgi?id=161805
 
index 6a3611a..6204526 100644 (file)
@@ -150,12 +150,12 @@ static bool isSpecialScheme(StringView scheme)
         || scheme == "wss";
 }
 
-static StringView bufferView(const StringBuilder& builder, unsigned length)
+static StringView bufferView(const StringBuilder& builder, unsigned start, unsigned length)
 {
     ASSERT(builder.length() >= length);
     if (builder.is8Bit())
-        return StringView(builder.characters8(), length);
-    return StringView(builder.characters16(), length);
+        return StringView(builder.characters8() + start, length);
+    return StringView(builder.characters16() + start, length);
 }
 
 enum class URLParser::URLPart {
@@ -236,7 +236,7 @@ void URLParser::copyURLPartsUntil(const URL& base, URLPart part)
         m_url.m_protocolIsInHTTPFamily = base.m_protocolIsInHTTPFamily;
         m_url.m_schemeEnd = base.m_schemeEnd;
     }
-    m_urlIsSpecial = isSpecialScheme(bufferView(m_buffer, m_url.m_schemeEnd));
+    m_urlIsSpecial = isSpecialScheme(bufferView(m_buffer, 0, m_url.m_schemeEnd));
 }
 
 static const char* dotASCIICode = "2e";
@@ -444,7 +444,7 @@ URL URLParser::parse(const String& input, const URL& base, const TextEncoding& e
                 m_buffer.append(toASCIILower(*c));
             else if (*c == ':') {
                 m_url.m_schemeEnd = m_buffer.length();
-                StringView urlScheme = bufferView(m_buffer, m_url.m_schemeEnd);
+                StringView urlScheme = bufferView(m_buffer, 0, m_url.m_schemeEnd);
                 m_url.m_protocolIsInHTTPFamily = urlScheme == "http" || urlScheme == "https";
                 if (urlScheme == "file") {
                     m_urlIsSpecial = true;
@@ -761,8 +761,7 @@ URL URLParser::parse(const String& input, const URL& base, const TextEncoding& e
                 if (!parseHost(authorityOrHostBegin, c))
                     return failure(input);
                 
-                // FIXME: Don't allocate a new string for this comparison.
-                if (m_buffer.toString().substring(m_url.m_passwordEnd) == "localhost")  {
+                if (bufferView(m_buffer, m_url.m_passwordEnd, m_buffer.length() - m_url.m_passwordEnd) == "localhost")  {
                     m_buffer.resize(m_url.m_passwordEnd);
                     m_url.m_hostEnd = m_buffer.length();
                     m_url.m_portEnd = m_url.m_hostEnd;
@@ -954,24 +953,19 @@ URL URLParser::parse(const String& input, const URL& base, const TextEncoding& e
             break;
         }
 
-        m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
-        m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
-        m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
-        m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
         if (!parseHost(authorityOrHostBegin, c))
             return failure(input);
         
-        // FIXME: Don't allocate a new string for this comparison.
-        if (m_buffer.toString().substring(m_url.m_passwordEnd) == "localhost")  {
+        if (bufferView(m_buffer, m_url.m_passwordEnd, m_buffer.length() - m_url.m_passwordEnd) == "localhost")  {
             m_buffer.resize(m_url.m_passwordEnd);
             m_url.m_hostEnd = m_buffer.length();
             m_url.m_portEnd = m_url.m_hostEnd;
-            m_buffer.append('/');
-            m_url.m_pathAfterLastSlash = m_url.m_hostEnd + 1;
-            m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
-            m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
-            m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
         }
+        m_buffer.append('/');
+        m_url.m_pathAfterLastSlash = m_url.m_hostEnd + 1;
+        m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
+        m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
+        m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
         break;
     case State::PathStart:
         LOG_FINAL_STATE("PathStart");
index c9c74df..25cf601 100644 (file)
@@ -1,3 +1,13 @@
+2016-09-09  Alex Christensen  <achristensen@webkit.org>
+
+        URLParser: Fix and optimize parsing file URLs ending with a host but no slash
+        https://bugs.webkit.org/show_bug.cgi?id=161815
+
+        Reviewed by Geoffrey Garen.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2016-09-08  Dean Jackson  <dino@apple.com>
 
         Expose Apple Pencil data to Touch events
index 9fc062b..8df5177 100644 (file)
@@ -157,6 +157,8 @@ TEST_F(URLParserTest, Basic)
     checkURL("file:////", {"file", "", "", "", 0, "//", "", "", "file:////"}); // This matches Firefox and URL::parse which I believe are correct, but not Chrome.
     checkURL("file:/path", {"file", "", "", "", 0, "/path", "", "", "file:///path"});
     checkURL("file://host/path", {"file", "", "", "host", 0, "/path", "", "", "file://host/path"});
+    checkURL("file://host", {"file", "", "", "host", 0, "/", "", "", "file://host/"});
+    checkURL("file://host/", {"file", "", "", "host", 0, "/", "", "", "file://host/"});
     checkURL("file:///path", {"file", "", "", "", 0, "/path", "", "", "file:///path"});
     checkURL("file:////path", {"file", "", "", "", 0, "//path", "", "", "file:////path"});
     checkURL("file://localhost/path", {"file", "", "", "", 0, "/path", "", "", "file:///path"});
@@ -423,6 +425,12 @@ TEST_F(URLParserTest, ParserDifferences)
     checkRelativeURLDifferences("notspecial:\\\\foo.com", "http://example.org/foo/bar",
         {"notspecial", "", "", "", 0, "\\\\foo.com", "", "", "notspecial:\\\\foo.com"},
         {"notspecial", "", "", "foo.com", 0, "", "", "", "notspecial://foo.com"});
+    checkURLDifferences("file://notuser:notpassword@test",
+        {"", "", "", "", 0, "", "", "", "file://notuser:notpassword@test"},
+        {"file", "notuser", "notpassword", "test", 0, "/", "", "", "file://notuser:notpassword@test/"});
+    checkURLDifferences("file://notuser:notpassword@test/",
+        {"", "", "", "", 0, "", "", "", "file://notuser:notpassword@test/"},
+        {"file", "notuser", "notpassword", "test", 0, "/", "", "", "file://notuser:notpassword@test/"});
     
     // This behavior matches Chrome and Firefox, but not WebKit using URL::parse.
     // The behavior of URL::parse is clearly wrong because reparsing file://path would make path the host.