Changing the styles to cause frame reconstruction can make the caret behave in a strange way in editable iframes

RESOLVED FIXED in Firefox -esr10

Status

()

Core
Editor
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Alfonso Martinez, Assigned: Ehsan)

Tracking

(Depends on: 1 bug)

Trunk
mozilla11
x86
All
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr1014+ verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 556391 [details]
Testcase

Given an iframe with <body contentEditable=true>, if some styles of a parent node are changed, then the iframe will have a strange behavior with the caret and the focus handling, it's possible to type in it, but using the arrow keys will "focus" the main page.

Other browsers behave as expected.

The testcase tries to explain better the situation.
Summary: Changing the styles can make the caret behave in a strange way in editable iframes → Changing the styles to cause frame reconstruction can make the caret behave in a strange way in editable iframes
Alfonso, does this bug affect a web page that you know of?
(Reporter)

Comment 2

6 years ago
Ups, I forgot to include a link to reference a CKEditor ticket: http://dev.ckeditor.com/ticket/8138

I found it while using one of the latests betas of my Write Area extension: https://addons.mozilla.org/en-US/firefox/addon/write-area/ in this case I've been able to workaround it by first setting a css rule on the page before creating the editor instead of changing the style of the element that I want to modify. Other people might not be aware of the problem.
Fabien, can you take a look at this, please?
Assignee: nobody → kaze

Comment 4

6 years ago
We have also seen this behavior and the immediate impact of hiding and showing the contenteditable iframe is that arrow key navigation within the area stops working correctly. 

Due to the keyboard navigation problems, this issue impacts accessibility.
Created attachment 575210 [details] [diff] [review]
Patch (v1)
Assignee: kaze → ehsan
Status: NEW → ASSIGNED
Attachment #575210 - Flags: review?(roc)

Comment 6

6 years ago
Try run for 33b6d0096764 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=33b6d0096764
Results (out of 190 total builds):
    exception: 3
    success: 165
    warnings: 18
    failure: 4
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-33b6d0096764
Attachment #575210 - Flags: review?(roc) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcc7876cab43
Flags: in-testsuite+
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/fcc7876cab43
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Comment on attachment 575210 [details] [diff] [review]
Patch (v1)

>--- a/layout/base/tests/test_reftests_with_caret.html
>+++ b/layout/base/tests/test_reftests_with_caret.html
>@@ -132,16 +132,17 @@ if (!isWindows) {
> 
> tests.push(function() {SpecialPowers.setBoolPref("bidi.browser.ui", true);});
> 
> if (!isWindows) {
>   tests.push([ 'bug646382-1.html' , 'bug646382-1-ref.html' ]);  // bug 681076
>   tests.push([ 'bug646382-2.html' , 'bug646382-2-ref.html' ]);  // bug 680577
>   tests.push([ 'bug664087-1.html' , 'bug664087-1-ref.html' ]);  // bug 681038
>   tests.push([ 'bug664087-2.html' , 'bug664087-2-ref.html' ]);  // bug 680578
>+  tests.push([ 'bug682712-1.html' , 'bug682712-1-ref.html' ]);  // disabled on Windows

Why is it disabled? Where is the bug?
(In reply to Ms2ger from comment #9)
> Comment on attachment 575210 [details] [diff] [review] [diff] [details] [review]
> Patch (v1)
> 
> >--- a/layout/base/tests/test_reftests_with_caret.html
> >+++ b/layout/base/tests/test_reftests_with_caret.html
> >@@ -132,16 +132,17 @@ if (!isWindows) {
> > 
> > tests.push(function() {SpecialPowers.setBoolPref("bidi.browser.ui", true);});
> > 
> > if (!isWindows) {
> >   tests.push([ 'bug646382-1.html' , 'bug646382-1-ref.html' ]);  // bug 681076
> >   tests.push([ 'bug646382-2.html' , 'bug646382-2-ref.html' ]);  // bug 680577
> >   tests.push([ 'bug664087-1.html' , 'bug664087-1-ref.html' ]);  // bug 681038
> >   tests.push([ 'bug664087-2.html' , 'bug664087-2-ref.html' ]);  // bug 680578
> >+  tests.push([ 'bug682712-1.html' , 'bug682712-1-ref.html' ]);  // disabled on Windows
> 
> Why is it disabled? Where is the bug?

There is a general problem with these tests on Windows (see the rest of the bugs referenced in that test file).  I have not had enough time to investigate what the issue is, and I didn't want to file more bugs which are ultimately going to be dupes of each other.  :-)
Depends on: 717868
Version: unspecified → Trunk
Depends on: 720626
No longer depends on: 717868

Comment 11

5 years ago
Wouldn't it make sense to also fix this in Firefox 10 ESR?

Comment 12

5 years ago
Thanks for fixing this in Firefox 11, works great!
However, I have to second Manuel's point. Bugs in CONTENTEDITABLE are a no-go especially for Enterprise customers who use the browser for intranet and content management applications. And we cannot tell our Enterprise customers to use Firefox 11. We were about approving our application for Firefox 10 ESR (everything else works fine), but this bug is a show-stopper. Thus, we desperately need an integrate of this fix in Firefox 10 ESR!
Nominating for ESR, but generally speaking ESR is supposed to only get security updates, so chances are somewhat slim....
tracking-firefox-esr10: --- → ?
[Triage Comment]
With things that aren't major stability/security issues we wait to see if there's a decent amount of user complaints and this doesn't seem to have any yet.  Please re-nominate for tracking if that changes.
tracking-firefox-esr10: ? → -

Comment 15

5 years ago
This bug affects CKEditor and presumably also all other WYSIWYG editors when used in non-trivial scenarios (hide/show), and thus a large user base. Such editors are often used in Enterprise Web applications, so exactly the clientele the ESR version is targeted at. Unfortunately, the bug fix in Firefox 11 does not really help us, as both we and our clients are bound to the ESR line. And: we are talking about a one line (production) code change, so the integrate should really be easy!

Comment 16

5 years ago
Any news on this?
My company is about to drop support for Firefox in favor of IE and Safari because of this bug.
Comment on attachment 575210 [details] [diff] [review]
Patch (v1)

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
   Seems to be biting several enterprise deployments of Firefox.
User impact if declined: Makes various web editors very hard to use.
Fix Landed on Version: Firefox 11.
Risk to taking this patch (and alternatives if risky): I believe this is low
   risk.  We've had no regression reports, and the patch is very simple.
String or UUID changes made by this patch: None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #575210 - Flags: approval-mozilla-esr10?
tracking-firefox-esr10: - → 14+
Comment on attachment 575210 [details] [diff] [review]
Patch (v1)

[Triage Comment]
low-risk and requested by enterprise users, approving for the ESR that will go out with FF14
Attachment #575210 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+

Comment 19

5 years ago
Great! Thank you very much, that helps a lot!
http://hg.mozilla.org/releases/mozilla-esr10/rev/e3fce7632667
status-firefox-esr10: --- → fixed
Verified fixed in Firefox 10.0.6esrpre 2012-06-21.
status-firefox-esr10: fixed → verified
You need to log in before you can comment on or make changes to this bug.