Closed
Bug 406596
Opened 17 years ago
Closed 17 years ago
Link/anchor elements are focused within an contentEditable element
Categories
(Core :: DOM: Editor, defect, P2)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla1.9
People
(Reporter: spam, Assigned: peterv)
References
()
Details
(Keywords: testcase)
Attachments
(3 files, 5 obsolete files)
(deleted),
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
jst
:
review+
jst
:
superreview+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b2pre) Gecko/2007120305 Minefield/3.0b2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b2pre) Gecko/2007120305 Minefield/3.0b2pre
When you enable contentEditable on an element, you are still able to focus the anchors within that element.
Reproducible: Always
Steps to Reproduce:
1. Open the attached URL.
2. Click the link within the editable area.
3. Observe that the link gets focused.
Actual Results:
Link is getting the focus with a dotted outline.
Expected Results:
The focus should still be on the editable area not inside the link.
Blocks: contenteditable
Version: unspecified → Trunk
Updated•17 years ago
|
Component: General → Editor
Product: Firefox → Core
QA Contact: general → editor
Comment 1•17 years ago
|
||
Does the link being focused cause problems, or is it just that it appears focused?
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9-
It's causing problems since it sends a blur event to the contentEditable container and the dotted border of the editable region becomes undotted and it's inconsistent with other contentEditable implementations.
This issue also affects existing designMode implementations like TinyMCE or FCKEditor, it at least adds an dotted focus border on links inside these areas but it doesn't seem to send the blur event but still I can not out rule that it breaks some existing code and it looks really strange. You can also tab to them inside the editable area and that is also a problem, they look focused but the focus isn't moved because if you type the content will be inserted at caret position.
Check this URL and click or tab to the links inside the editable area:
http://tinymce.moxiecode.com/example_full.php?example=true
I really think this one needs to be blocking now since it affects existing implementations it's one thing if it only produces problems with the new contentEditable feature but another thing it changes the behavior of existing code.
Flags: blocking1.9- → blocking1.9?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
Assignee: nobody → chris
Comment 4•17 years ago
|
||
Comment 3 is implying this is a regression from 1.8 at least for designMode. Is that the case?
Assignee | ||
Comment 5•17 years ago
|
||
Need to do at least this, to fix designmode. Fixing it for contentEditable is trickier, need to think a bit about interactions with tabindex etc. And most other browsers allow focusing at least some elements (so they get a focus ring), though not anchor elements I think.
Assignee: chris → peterv
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 6•17 years ago
|
||
Comment on attachment 301953 [details] [diff] [review]
v1
Stop focussing anything except the document in designMode.
Attachment #301953 -
Flags: superreview?(jst)
Attachment #301953 -
Flags: review?(jst)
Updated•17 years ago
|
Attachment #301953 -
Flags: superreview?(jst)
Attachment #301953 -
Flags: superreview+
Attachment #301953 -
Flags: review?(jst)
Attachment #301953 -
Flags: review+
Flags: wanted1.9+ → blocking1.9+
Priority: P3 → P2
Updated•17 years ago
|
Flags: tracking1.9+
Comment 8•17 years ago
|
||
Peter - how we doin on this bug?
Assignee | ||
Comment 9•17 years ago
|
||
Comment 10•17 years ago
|
||
Peter we are nearing RC1 freeze. Is this ready to go or should we punt to .next?
Assignee | ||
Comment 11•17 years ago
|
||
It's not ready to go, I'm still debugging why the caret disappears in the anchor element.
Assignee | ||
Comment 12•17 years ago
|
||
I think I have a fix for this and for bug 421640, turns out they're related. Still need to clean up and do a bit of testing.
Reporter | ||
Comment 13•17 years ago
|
||
That sounds super nice since I think this and 421640 is the most serious ones they are show stoppers for one of our applications. Keep up the good work. :)
Assignee | ||
Comment 14•17 years ago
|
||
This is looking pretty good. Still a minor issue when tabbing from a contentEditable link.
Attachment #310327 -
Attachment is obsolete: true
Assignee | ||
Comment 15•17 years ago
|
||
Attachment #314421 -
Attachment is obsolete: true
Comment 16•17 years ago
|
||
We wouldn't hold the release for this if it were the last bug on the list (we're at that point now). Moving to wanted1.9.0.x+.
Flags: wanted1.9.0.x+
Flags: blocking1.9-
Flags: blocking1.9+
Assignee | ||
Comment 17•17 years ago
|
||
Including mochitest for this and bug 421640. Still tracking down an issue with a disappearing selection when tabbing. Really close now.
Attachment #314676 -
Attachment is obsolete: true
Assignee | ||
Comment 18•17 years ago
|
||
This one looks good to go.
Attachment #314976 -
Attachment is obsolete: true
Assignee | ||
Comment 19•17 years ago
|
||
Make anchor elements non-focusable if they're in a contenteditable region, except if they are a contenteditable root. Don't clear the selection when focus moves out of a contenteditable region. Instead, make the event state manager not start from the selection when tabbing in a document where an editable contenteditable element is focused. Only execute the code in nsTextEditorFocusListener::Blur for the nsTextEditorFocusListener of the editor that was focussed last (there can be multiple editors and so multiple nsTextEditorFocusListeners per document).
Attachment #314994 -
Attachment is obsolete: true
Attachment #315189 -
Flags: superreview?
Attachment #315189 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #315189 -
Flags: superreview?(jst)
Attachment #315189 -
Flags: superreview?
Attachment #315189 -
Flags: review?(jst)
Attachment #315189 -
Flags: review?
Comment 20•17 years ago
|
||
Comment on attachment 315189 [details] [diff] [review]
Fix anchor contenteditable elements v2.4
r+sr=jst
Attachment #315189 -
Flags: superreview?(jst)
Attachment #315189 -
Flags: superreview+
Attachment #315189 -
Flags: review?(jst)
Attachment #315189 -
Flags: review+
Assignee | ||
Comment 21•17 years ago
|
||
Comment on attachment 315189 [details] [diff] [review]
Fix anchor contenteditable elements v2.4
This patch fixes a number of bugs:
- this bug ( wanted1.9.0.x)
- bug 413678 (wanted‑next)
- bug 418135
- bug 421640 ( blocking1.9)
We have had regressions from previous fixes for contenteditable focus/caret fixes, so this isn't risk-free. However, I think the approach here is quite sound, I've run the whole mochitest suite and I've added tests for this bug and bug 421640. I'll try to convert the testcase from bug 413678 to an automated test (but don't think we can do that for the one from bug 418135).
Attachment #315189 -
Flags: approval1.9?
Comment 22•17 years ago
|
||
Comment on attachment 315189 [details] [diff] [review]
Fix anchor contenteditable elements v2.4
a=beltzner, please do continue to file those tests, but let's get this in earlier so that any regressions (fingers crossed!) get on our radars earlier.
Attachment #315189 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
OS: Windows Vista → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
Assignee | ||
Updated•17 years ago
|
Flags: in-testsuite+
Comment 23•17 years ago
|
||
(In reply to comment #21)
> (From update of attachment 315189 [details] [diff] [review])
> This patch fixes a number of bugs:
> - this bug ( wanted1.9.0.x)
> - bug 413678 (wanted‑next)
> - bug 418135
> - bug 421640 ( blocking1.9)
>
> We have had regressions from previous fixes for contenteditable focus/caret
> fixes, so this isn't risk-free. However, I think the approach here is quite
> sound, I've run the whole mochitest suite and I've added tests for this bug and
> bug 421640. I'll try to convert the testcase from bug 413678 to an automated
> test (but don't think we can do that for the one from bug 418135).
>
Could bug 419808 also be fixed by this?
Comment 24•17 years ago
|
||
This broke most Flash text input in Camino; see bug 430351. (Given the timing and behavior, it may also be responsible for bug 429771, but that hasn't been tested yet)
Updated•16 years ago
|
Flags: wanted1.9.0.x+
You need to log in
before you can comment on or make changes to this bug.
Description
•