692153 - contenteditable=true breaks copy menuitem

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[dindog]

Attachment: testcase

When there is contenteditable=true element, Copy and Select All menuitem gray-out when selected node is text.

It should block Bug 237964.

It's a regression first appearred in 20110918's nightly.

Comment 1

[(no longer active)]

Alice, can you please see if you can determine a regression range for this?  Thanks!  :-)

Comment 2

[Alice0775 White]

Regression window for the problem of "Copy and Select All menuitem gray-out"

Regression winow(m-c),
Works:
http://hg.mozilla.org/mozilla-central/rev/8b970cb862f2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110817 Firefox/9.0a1 ID:20110818000349
Fails:
http://hg.mozilla.org/mozilla-central/rev/f69a10f23bf3
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110818 Firefox/9.0a1 ID:20110818030747
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8b970cb862f2&tochange=f69a10f23bf3


Regression winow(m-i),
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/e6d269bc23b9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110817 Firefox/9.0a1 ID:20110817074955
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/05268baefef7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110817 Firefox/9.0a1 ID:20110817115336
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e6d269bc23b9&tochange=05268baefef7

Suspected bug:
Bug 676401 - The queryCommandEnabled API doesn't take the active editing host into account

Comment 3

[dindog]

any news about this bug?
It affects Fx9+(current beta), if we don't fix it in Fx11, users have to wait at least 3*6=18 weeks before thay can copy text in those page again...

Comment 4

[Mozilla RelEng Bot]

Try run for c1e1cdb0e7d6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=c1e1cdb0e7d6
Results (out of 207 total builds):
    success: 160
    warnings: 46
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-c1e1cdb0e7d6

Comment 5

[Alice0775 White]

The try server build seems to fix only "Copy".

"Select All" is still broken....

Comment 6

[[e-mail]]

(In reply to Alice White from comment #5)
> The try server build seems to fix only "Copy".
> 
> "Select All" is still broken....

It was only supposed to fix "Copy". I really wanted to see how many tests fail.

Comment 7

[[e-mail]]

OK, new approach coming up, this one seems to fix both Copy and Select All :-)

Comment 8

[[e-mail]]

Attachment: Proposed patch

Fortunately Copy and Select All aren't affected by caret browsing.

Comment 9

[Mozilla RelEng Bot]

Try run for fb3e6d2977ae is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=fb3e6d2977ae
Results (out of 209 total builds):
    success: 172
    warnings: 37
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-fb3e6d2977ae

Comment 10

[Mozilla RelEng Bot]

Try run for 54a00d6d26c2 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=54a00d6d26c2
Results (out of 24 total builds):
    success: 22
    warnings: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-54a00d6d26c2

Comment 11

[(no longer active)]

Comment on attachment 584296 [details] [diff] [review]
Proposed patch

When I was writing https://bugzilla.mozilla.org/attachment.cgi?id=560350, I knew that I had seen a separate bug on this somewhere!  :-)

r=me

Comment 12

[[e-mail]]

Pushed changeset 4dd80e8ee29e to mozilla-central.

Comment 16

[James Pearson]

This bug is also in Firefox ESR 10.0

This bug is not critical or high, nor causes a crash, but a regression that appeared in v9.0 and was fixed in v12.0.

It results in unpredictable behaviour for users when trying to copy text from webpages - sometimes the copy menu is greyed out, sometimes it is not

The patch that fixes this bug applies cleanly to the 10.0 ESR source (and does indeed fix the issue for 10.0 ESR)

I would appreciated it if this patch can be applied to the ESR tree

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

[Triage Comment]
Looks like a low-risk fix and we're happy to take the occasional usability fix when requested by actual users so please go ahead and nominated for ESR landing.

Comment 18

[(no longer active)]

Comment on attachment 584296 [details] [diff] [review]
Proposed patch

[Approval Request Comment]
User impact if declined: See comment 16.
Fix Landed on Version: Firefox 12.
Risk to taking this patch (and alternatives if risky): Shouldn't be too risky.
String or UUID changes made by this patch:  None.

Comment 19

[[e-mail]]

Pushed mozilla-esr10 changeset 445a090fef3f.

Comment 20

[u279076]

Verified fixed in Firefox 10.0.6esrpre 2012-06-21 using the attached testcase.