Closed Bug 682712 Opened 13 years ago Closed 13 years ago

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

Categories

(Core :: DOM: Editor, defect)

Product:
Core
Component:
DOM: Editor
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla11
Tracking Flags:
Tracking Status
firefox-esr10 14+ verified

People

(Reporter: amla70, Assigned: ehsan.akhgari)

References

Details

Attachments

(2 files)

Testcase
1.85 KB, text/html
Details
Patch (v1)
5.01 KB, patch
roc
: review+
lsblakk
: approval-mozilla-esr10+
Details | Diff | Splinter Review
Attached file 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?
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
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.
Attached patch Patch (v1) — — Splinter Review
Assignee: kaze → ehsan
Status: NEW → ASSIGNED
Attachment #575210 - Flags: review?(roc)
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
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
Closed: 13 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
Wouldn't it make sense to also fix this in Firefox 10 ESR?
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: ? → -
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!
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?
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+
Great! Thank you very much, that helps a lot!
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.

Attachment

General

Creator:
Created:
Updated:
Size: