Last Comment Bug 682712 - Changing the styles to cause frame reconstruction can make the caret behave in a strange way in editable iframes
: Changing the styles to cause frame reconstruction can make the caret behave i...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: x86 All
: -- normal with 2 votes (vote)
: mozilla11
Assigned To: :Ehsan Akhgari
:
Mentors:
Depends on: 720626
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-28 07:33 PDT by Alfonso Martinez
Modified: 2012-06-21 15:28 PDT (History)
7 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
14+
verified


Attachments
Testcase (1.85 KB, text/html)
2011-08-28 07:33 PDT, Alfonso Martinez
no flags Details
Patch (v1) (5.01 KB, patch)
2011-11-17 09:17 PST, :Ehsan Akhgari
roc: review+
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description Alfonso Martinez 2011-08-28 07:33:55 PDT
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.
Comment 1 :Ehsan Akhgari 2011-09-07 13:23:16 PDT
Alfonso, does this bug affect a web page that you know of?
Comment 2 Alfonso Martinez 2011-09-07 13:30:56 PDT
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.
Comment 3 :Ehsan Akhgari 2011-09-07 16:13:39 PDT
Fabien, can you take a look at this, please?
Comment 4 Damian 2011-11-17 03:46:25 PST
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.
Comment 5 :Ehsan Akhgari 2011-11-17 09:17:00 PST
Created attachment 575210 [details] [diff] [review]
Patch (v1)
Comment 6 Mozilla RelEng Bot 2011-11-17 13:20:29 PST
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
Comment 8 Ed Morley [:emorley] 2011-11-19 05:12:41 PST
https://hg.mozilla.org/mozilla-central/rev/fcc7876cab43
Comment 9 :Ms2ger (⌚ UTC+1/+2) 2011-11-20 11:28:53 PST
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?
Comment 10 :Ehsan Akhgari 2011-11-21 11:50:26 PST
(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.  :-)
Comment 11 manuel.ohlendorf 2012-03-05 01:05:15 PST
Wouldn't it make sense to also fix this in Firefox 10 ESR?
Comment 12 Frank Wienberg 2012-03-16 03:40:22 PDT
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!
Comment 13 Boris Zbarsky [:bz] (TPAC) 2012-03-16 09:11:11 PDT
Nominating for ESR, but generally speaking ESR is supposed to only get security updates, so chances are somewhat slim....
Comment 14 Lukas Blakk [:lsblakk] use ?needinfo 2012-03-16 13:10:42 PDT
[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.
Comment 15 Frank Wienberg 2012-03-16 16:43:16 PDT
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 Frank Wienberg 2012-06-11 06:41:28 PDT
Any news on this?
My company is about to drop support for Firefox in favor of IE and Safari because of this bug.
Comment 17 Boris Zbarsky [:bz] (TPAC) 2012-06-11 20:42:31 PDT
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.
Comment 18 Lukas Blakk [:lsblakk] use ?needinfo 2012-06-14 14:59:21 PDT
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
Comment 19 Frank Wienberg 2012-06-15 01:30:29 PDT
Great! Thank you very much, that helps a lot!
Comment 20 Boris Zbarsky [:bz] (TPAC) 2012-06-15 12:32:46 PDT
http://hg.mozilla.org/releases/mozilla-esr10/rev/e3fce7632667
Comment 21 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-06-21 15:28:19 PDT
Verified fixed in Firefox 10.0.6esrpre 2012-06-21.

Note You need to log in before you can comment on or make changes to this bug.