Summary: | REGRESSION (r102553): Smart links do not work | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | adele, ddavidso, dglazkov, enrica, jbedard, jberlin, jiapu.mail, max.hong.shen, morrita, rakuco, rniwa, shinyak, tkent, webkit.review.bot | ||||||||||
Priority: | P1 | Keywords: | InRadar, Regression | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
URL: | data:text/html,%3Cdiv%20contenteditable%3E%3C/div%3E%3Cscript%3Edocument.getElementsByTagName(%22div%22)%5B0%5D.focus()%3C/script%3E | ||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=161590 https://bugs.webkit.org/show_bug.cgi?id=162081 |
||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 82198 | ||||||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2012-05-02 23:58:38 PDT
Other substitutions appear to work fine. Regressed in http://trac.webkit.org/changeset/102553 To fix: https://bugs.webkit.org/show_bug.cgi?id=73616 "Asynchronous path and synchronous path of SpellChecker should share the code to mark misspellings" Is there anyway to test smart links in DRT? I would expect that capturing its working is possible with textInputController. It's relatively easy to see the calls if you enable TextInput log channel. Created attachment 141737 [details]
simple fix
Comment on attachment 141737 [details] simple fix View in context: https://bugs.webkit.org/attachment.cgi?id=141737&action=review > Source/WebCore/ChangeLog:11 > + No new tests - fix the regression Is it possible to write a layout test for this? If not, should we add a manual test? (In reply to comment #7) > (From update of attachment 141737 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=141737&action=review > > > Source/WebCore/ChangeLog:11 > > + No new tests - fix the regression > > Is it possible to write a layout test for this? If not, should we add a manual test? Maybe ap would know? Comment on attachment 141737 [details] simple fix View in context: https://bugs.webkit.org/attachment.cgi?id=141737&action=review >>> Source/WebCore/ChangeLog:11 >>> + No new tests - fix the regression >> >> Is it possible to write a layout test for this? If not, should we add a manual test? > > Maybe ap would know? I don't know how to exercise this code in a regression test. "fix the regression" is not a good explanation of why there are no tests though. Thanks for the comments. To enable the smart link test in DRT, we need to add a new interface like setLinkDetectionEnabled in LayoutTestController. I can open a separated bug and work on it. Created attachment 142304 [details]
add a manual test
To enable the layouttest for the smart link, it requires adding a new API in WebView which may take a while for discussing. In addition, Smart links is not included in w3c spec, and I am not sure whether we should have it in LayoutTest.
So, add a manual test for this small patch to help passing the review :)
(In reply to comment #11) > Created an attachment (id=142304) [details] > add a manual test > > To enable the layouttest for the smart link, it requires adding a new API in WebView which may take a while for discussing. In addition, Smart links is not included in w3c spec, and I am not sure whether we should have it in LayoutTest. I don't think whether something is included in W3C spec or not is a good indication as to whether we should write a test or not since layout tests aren't conformance tests. Created attachment 142485 [details]
add layouttest
Attachment 142485 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/edit..." exit_code: 1
Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:852: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5]
Tools/DumpRenderTree/LayoutTestController.cpp:1724: Tab found; better to use spaces [whitespace/tab] [1]
Tools/DumpRenderTree/wx/LayoutTestControllerWx.cpp:649: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5]
Tools/DumpRenderTree/win/LayoutTestControllerWin.cpp:1524: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5]
Total errors found: 4 in 22 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 142485 [details] add layouttest Attachment 142485 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12725150 Comment on attachment 142485 [details] add layouttest Attachment 142485 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12726163 Comment on attachment 142485 [details] add layouttest Attachment 142485 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12717564 Created attachment 142497 [details]
fix style and build issues
Comment on attachment 142497 [details]
fix style and build issues
Thanks for the test!
(In reply to comment #19) > (From update of attachment 142497 [details]) > Thanks for the test! Thanks for the review :) Comment on attachment 142497 [details] fix style and build issues Rejecting attachment 142497 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: saving rejects to file LayoutTests/platform/efl/test_expectations.txt.rej patching file LayoutTests/platform/gtk/test_expectations.txt patching file LayoutTests/platform/qt/Skipped Hunk #1 succeeded at 2480 (offset -4 lines). patching file LayoutTests/platform/qt/test_expectations.txt patching file LayoutTests/platform/win/Skipped Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Ryosuke Ni..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/12719568 Comment on attachment 142497 [details] fix style and build issues Rejecting attachment 142497 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: pectations.txt patching file LayoutTests/platform/qt/Skipped Hunk #1 succeeded at 2480 (offset -4 lines). patching file LayoutTests/platform/qt/test_expectations.txt Hunk #1 FAILED at 72. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/qt/test_expectations.txt.rej patching file LayoutTests/platform/win/Skipped Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Ryosuke Ni..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/12722765 Committed r117590: <http://trac.webkit.org/changeset/117590> Please try to remember WKTR when you add something to DRT: http://build.webkit.org/results/Lion%20Release%20(WebKit2%20Tests)/r118030%20(7431)/editing/inserting/typing-space-to-trigger-smart-link-pretty-diff.html I will be adding the test to the wk2 skipped list. Thanks for the help! (In reply to comment #24) > Please try to remember WKTR when you add something to DRT: http://build.webkit.org/results/Lion%20Release%20(WebKit2%20Tests)/r118030%20(7431)/editing/inserting/typing-space-to-trigger-smart-link-pretty-diff.html > I will be adding the test to the wk2 skipped list. |