Closed
Bug 1061468
Opened 10 years ago
Closed 10 years ago
Focus on div element after DOM manipulation
Categories
(Core :: DOM: Editor, defect)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: lonevvolf, Assigned: MatsPalmgren_bugz)
Details
(Keywords: testcase)
Attachments
(3 files)
428 bytes,
text/html
|
Details | |
9.83 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
3.47 KB,
patch
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/37.0.2062.102 Safari/537.36 Steps to reproduce: Create a contenteditable element with focus. Programmatically move the element to another position. Focus the element again (or not). Actual results: Caret and focus stays in the old position, even if focus() is called again. Expected results: Caret and focus should have been on the newly positioned element.
Updated•10 years ago
|
Component: Untriaged → DOM
Product: Firefox → Core
Updated•10 years ago
|
Flags: needinfo?(ehsan.akhgari)
Comment 1•10 years ago
|
||
I actually think that this is the same underlying issue as bug 1060579. Bug 1060579 comment 6 suggests a fix. I wasn't thinking about nsCaret relying on selection listeners, we should probably fix this. Olli, do you happen to have the cycles?
Depends on: 1060579
Flags: needinfo?(ehsan.akhgari) → needinfo?(bugs)
Comment 2•10 years ago
|
||
Don't have time right now, and Mats might actually have this code in mind given the other patches he has written lately.
Flags: needinfo?(bugs)
Assignee | ||
Comment 4•10 years ago
|
||
I don't think this is related to bug 1060579 at all. IMO, this is a bug in Editor since it depends on blur events to shutdown the selection (FinalizeSelection). We don't blur the focused element when it's removed from the document (unlike Chrome). (fwiw, I think Chrome's behavior actually makes sense here.) What happens is that the Range gravitates when the node is removed, resulting in its start/end container being the parent of the ancestor-limiter that Editor assigned. We could make Ranges aware of the limiter and not gravitate above that point, but then we'll have invalid (disconnected) Ranges instead, which isn't pleasant either. And it wouldn't be spec compliant, nor UA compatible (IE, Presto, Chrome gravitates too). So, I'd prefer to fix this by detecting this specific case and call FinalizeSelection on the editor as if the element was blur-ed.
Assignee | ||
Comment 5•10 years ago
|
||
I piggy-backed nsFocusManager::ContentRemoved since it already has code to detect when the focused element is removed (and we're only interested in the focus case since Editor::Blur takes care of it otherwise). Alternatively, we could make nsEditor observe mutations but it would be slower.
Assignee: nobody → mats
Attachment #8486068 -
Flags: review?(ehsan.akhgari)
Assignee | ||
Comment 6•10 years ago
|
||
Try run (that also included bug 739396): https://tbpl.mozilla.org/?tree=Try&rev=dc301974fe74 https://tbpl.mozilla.org/?tree=Try&rev=b1ba440f2188
Comment 7•10 years ago
|
||
Comment on attachment 8486068 [details] [diff] [review] fix Review of attachment 8486068 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/public/nsISelectionPrivate.idl @@ +85,1 @@ > [noscript] void setAncestorLimiter(in nsIContent aContent); Nit: should we instead have: [noscript] attribute nsIContent ancestorLimiter; ::: dom/base/nsFocusManager.cpp @@ +849,5 @@ > + selection->GetAncestorLimiter(getter_AddRefs(limiter)); > + if (limiter == content) { > + editor->FinalizeSelection(); > + } > + } Nit: trailing whitespace. ::: editor/nsIEditor.idl @@ +44,5 @@ > > /** > + * Finalizes selection and caret for the editor. > + */ > + void finalizeSelection(); Nit: [noscript], please.
Attachment #8486068 -
Flags: review?(ehsan.akhgari) → review+
Assignee | ||
Comment 8•10 years ago
|
||
With nits addressed as suggested: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/004917420079 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/0ba3143c153c
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/004917420079 https://hg.mozilla.org/mozilla-central/rev/0ba3143c153c
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•