Bug 118287 - SelectAll on a page does not work if first/last element within the body is contenteditable
Summary: SelectAll on a page does not work if first/last element within the body is co...
Status: UNCONFIRMED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-02 02:35 PDT by vanivhegde
Modified: 2013-11-25 01:33 PST (History)
8 users (show)

See Also:


Attachments
test.html (270 bytes, text/html)
2013-07-02 02:35 PDT, vanivhegde
no flags Details
Patch (4.83 KB, patch)
2013-07-02 04:49 PDT, vanivhegde
no flags Details | Formatted Diff | Diff
WIP Patch (4.56 KB, patch)
2013-07-02 05:11 PDT, vanivhegde
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (617.00 KB, application/zip)
2013-07-02 06:18 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (566.41 KB, application/zip)
2013-07-02 09:10 PDT, Build Bot
no flags Details
Archive of layout-test-results from APPLE-EWS-1 for win-future (899.14 KB, application/zip)
2013-07-03 16:09 PDT, Build Bot
no flags Details
WIP Patch (6.53 KB, patch)
2013-07-08 06:21 PDT, vanivhegde
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (490.08 KB, application/zip)
2013-07-08 08:16 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (543.40 KB, application/zip)
2013-07-08 10:04 PDT, Build Bot
no flags Details
Patch (15.35 KB, patch)
2013-07-09 07:38 PDT, vanivhegde
no flags Details | Formatted Diff | Diff
Patch (15.40 KB, patch)
2013-07-10 06:52 PDT, vanivhegde
rniwa: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (521.36 KB, application/zip)
2013-07-10 21:02 PDT, Build Bot
no flags Details
Work in progress (1.62 KB, patch)
2013-09-29 01:22 PDT, Santosh Mahto
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (473.32 KB, application/zip)
2013-09-29 02:11 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion (487.91 KB, application/zip)
2013-09-29 02:31 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (487.35 KB, application/zip)
2013-09-29 03:29 PDT, Build Bot
no flags Details
Work in progress (2.67 KB, patch)
2013-09-29 03:36 PDT, Santosh Mahto
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 (472.58 KB, application/zip)
2013-09-29 04:49 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (490.80 KB, application/zip)
2013-09-29 05:11 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (486.70 KB, application/zip)
2013-09-29 06:08 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description vanivhegde 2013-07-02 02:35:57 PDT
Created attachment 205880 [details]
test.html

I have attached the test html.

Steps to Reproduce:
Load the test html
Click on SelectAll button OR do CTRL+A

Expected Result:
The entire page should be selected

Actual Result:
Page does not get selected


NOTE:
Please note that this happens only when the first element in body is content editable

The test html source is as below:
<html>
	<head>
		<script type="text/javascript">function SelectAll(){document.execCommand('selectAll', false, null);}</script>
	</head>
	<body>
		<div contenteditable=true>Select this text</div>
		<button onclick="SelectAll()">SelectAll</button>
	</body>
</html>

Had there been another div without contenteditable property set, before the contenteditable div, select all works just fine
i.e. this works
	<body>
                <div>Select this text</div>
		<div contenteditable=true>Select this text</div>
		<button onclick="SelectAll()">SelectAll</button>
	</body>
Comment 1 vanivhegde 2013-07-02 04:49:06 PDT
Created attachment 205892 [details]
Patch
Comment 2 vanivhegde 2013-07-02 05:11:42 PDT
Created attachment 205895 [details]
WIP Patch
Comment 3 vanivhegde 2013-07-02 05:33:22 PDT
Please note that this is a WIP patch. I am looking for some inputs to arrive at the fix. Please have a look.

Below are the regressions caused:

Changes in VisiblePosition.cpp cause below regressions:
  editing/deleting/5272440.html [ Failure ]
  editing/execCommand/crash-indenting-list-item.html [ Failure ]
  editing/selection/click-outside-editable-div.html [ Failure ]


Changes in VisibleSelection.cpp cause below regressions:
Regressions: Unexpected image and text failures (1)
  editing/selection/editable-non-editable-crash.html [ Failure ]

Regressions: Unexpected text-only failures (1)
  editing/execCommand/indent-pre-list.html [ Failure ]


Changes in FrameSelection.cpp cause below regressions:
  editing/selection/select-all-006.html [ Failure ]
  editing/style/apple-style-editable-mix.html [ Failure ]
Comment 4 Build Bot 2013-07-02 06:18:35 PDT
Comment on attachment 205895 [details]
WIP Patch

Attachment 205895 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1016327

New failing tests:
editing/execCommand/indent-pre-list.html
editing/deleting/5272440.html
editing/style/apple-style-editable-mix.html
editing/selection/select-all-006.html
editing/execCommand/crash-indenting-list-item.html
editing/selection/editable-non-editable-crash.html
fast/dom/remove-body-during-body-replacement2.html
editing/selection/click-outside-editable-div.html
Comment 5 Build Bot 2013-07-02 06:18:37 PDT
Created attachment 205900 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.3
Comment 6 Build Bot 2013-07-02 09:10:12 PDT
Comment on attachment 205895 [details]
WIP Patch

Attachment 205895 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/927959

New failing tests:
editing/execCommand/indent-pre-list.html
editing/deleting/5272440.html
editing/style/apple-style-editable-mix.html
editing/selection/select-all-006.html
editing/execCommand/crash-indenting-list-item.html
editing/selection/editable-non-editable-crash.html
fast/dom/remove-body-during-body-replacement2.html
editing/selection/click-outside-editable-div.html
Comment 7 Build Bot 2013-07-02 09:10:14 PDT
Created attachment 205921 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 8 Build Bot 2013-07-03 16:09:01 PDT
Comment on attachment 205895 [details]
WIP Patch

Attachment 205895 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/1024180

New failing tests:
editing/execCommand/indent-pre-list.html
editing/deleting/5272440.html
editing/execCommand/crash-indenting-list-item.html
editing/selection/editable-non-editable-crash.html
fast/dom/remove-body-during-body-replacement2.html
editing/selection/click-outside-editable-div.html
Comment 9 Build Bot 2013-07-03 16:09:04 PDT
Created attachment 206033 [details]
Archive of layout-test-results from APPLE-EWS-1 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-1  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit
Comment 10 vanivhegde 2013-07-08 06:21:54 PDT
Created attachment 206238 [details]
WIP Patch
Comment 11 Build Bot 2013-07-08 08:16:08 PDT
Comment on attachment 206238 [details]
WIP Patch

Attachment 206238 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1008172

New failing tests:
fullscreen/full-screen-iframe-with-max-width-height.html
editing/execCommand/crash-indenting-list-item.html
Comment 12 Build Bot 2013-07-08 08:16:10 PDT
Created attachment 206246 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 13 Build Bot 2013-07-08 10:04:36 PDT
Comment on attachment 206238 [details]
WIP Patch

Attachment 206238 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1037148

New failing tests:
editing/execCommand/crash-indenting-list-item.html
Comment 14 Build Bot 2013-07-08 10:04:39 PDT
Created attachment 206256 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.3
Comment 15 vanivhegde 2013-07-09 07:38:36 PDT
Created attachment 206321 [details]
Patch
Comment 16 vanivhegde 2013-07-10 06:52:01 PDT
Created attachment 206388 [details]
Patch
Comment 17 Build Bot 2013-07-10 21:02:10 PDT
Comment on attachment 206388 [details]
Patch

Attachment 206388 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/881989

New failing tests:
fullscreen/full-screen-iframe-with-max-width-height.html
Comment 18 Build Bot 2013-07-10 21:02:13 PDT
Created attachment 206425 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 19 vanivhegde 2013-07-11 04:50:43 PDT
fullscreen/full-screen-iframe-with-max-width-height.html
This doesn't seems to be a failure related to editing. This is found to be flacky even in the latest revision 152561.
Seems like this exists since r152434 
Reference: http://build.webkit.org/TestFailures/#/Apple MountainLion Release WK2 (Tests)
Comment 20 Ryosuke Niwa 2013-07-11 21:04:43 PDT
Comment on attachment 206388 [details]
Patch

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

> Source/WebCore/ChangeLog:20
> +        Also, executing selectAll without focusing anywhere on the page
> +        should not set focus on a contenteditable element if it happens to be
> +        the first/last element within the body.

This should be fixed as a separate bug. r- because due to this unrelated change.

> Source/WebCore/editing/VisiblePosition.cpp:557
> +    // If the html element is non-editable and next/previous candidates have editable roots,
> +    // then consider them to be in same editable element
> +    if (!editingRoot && node && node->hasTagName(htmlTag)) {
> +        prevIsInSameEditableElement = prevNode && !prevNode->isRootEditableElement();
> +        nextIsInSameEditableElement = nextNode && !nextNode->isRootEditableElement();
> +    } else {

Why? This is what comment, which is obvious from the code.
We need to explain why we do this. In fact, this doesn't look right.

> Source/WebCore/editing/VisibleSelection.cpp:567
> +    // If selection start & end positions are first & last elements of the body respectively,
> +    // it means the entire page is selected. No adjustments required.

Doesn't. You can make head element visible. You can also insert a random element before/after body and make it editable.
r- because of this wrong assumption as well.
Comment 21 Santosh Mahto 2013-09-29 01:22:37 PDT
Created attachment 212918 [details]
Work in progress
Comment 22 Build Bot 2013-09-29 02:11:31 PDT
Comment on attachment 212918 [details]
Work in progress

Attachment 212918 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/2740086

New failing tests:
editing/execCommand/crash-indenting-list-item.html
editing/selection/click-outside-editable-div.html
Comment 23 Build Bot 2013-09-29 02:11:33 PDT
Created attachment 212919 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 24 Build Bot 2013-09-29 02:31:32 PDT
Comment on attachment 212918 [details]
Work in progress

Attachment 212918 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/2691097

New failing tests:
editing/execCommand/crash-indenting-list-item.html
editing/selection/click-outside-editable-div.html
Comment 25 Build Bot 2013-09-29 02:31:34 PDT
Created attachment 212920 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 26 Build Bot 2013-09-29 03:29:36 PDT
Comment on attachment 212918 [details]
Work in progress

Attachment 212918 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/2716149

New failing tests:
editing/execCommand/crash-indenting-list-item.html
editing/selection/click-outside-editable-div.html
Comment 27 Build Bot 2013-09-29 03:29:39 PDT
Created attachment 212921 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 28 Santosh Mahto 2013-09-29 03:36:00 PDT
Created attachment 212922 [details]
Work in progress
Comment 29 Build Bot 2013-09-29 04:49:43 PDT
Comment on attachment 212922 [details]
Work in progress

Attachment 212922 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/2703042

New failing tests:
editing/execCommand/crash-indenting-list-item.html
editing/selection/click-outside-editable-div.html
Comment 30 Build Bot 2013-09-29 04:49:45 PDT
Created attachment 212923 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 31 Build Bot 2013-09-29 05:11:10 PDT
Comment on attachment 212922 [details]
Work in progress

Attachment 212922 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/2755005

New failing tests:
editing/execCommand/crash-indenting-list-item.html
editing/selection/click-outside-editable-div.html
Comment 32 Build Bot 2013-09-29 05:11:13 PDT
Created attachment 212924 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 33 Build Bot 2013-09-29 06:08:37 PDT
Comment on attachment 212922 [details]
Work in progress

Attachment 212922 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/2760017

New failing tests:
editing/execCommand/crash-indenting-list-item.html
editing/selection/click-outside-editable-div.html
Comment 34 Build Bot 2013-09-29 06:08:42 PDT
Created attachment 212927 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 35 Ryosuke Niwa 2013-09-29 20:21:54 PDT
Could you please make sure the following two tests pass with your patch BEFORE uploading it here?

editing/execCommand/crash-indenting-list-item.html
editing/selection/click-outside-editable-div.html

It has been very noisy due to these test failures.