Closed
Bug 694880
Opened 13 years ago
Closed 13 years ago
The editable state is not updated correctly when designMode is turned off
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(2 files)
See the attached test case. The bug is caused because of the incorrect assumption made by NotifyEditableStateChange. This function assumes that when designMode is turned on, we propagate the NODE_IS_EDITABLE flag throughout the document, so it tries to optimize not calling UpdateState if the value of the flag has not changed. When the designMode is turned off, none of the nodes will have the NODE_IS_EDITABLE_FLAG, so the check will always fail, and UpdateState will never be called, leaving all of the nodes in a document with an incorrect state.
Assignee | ||
Comment 1•13 years ago
|
||
Also, if there is no contentEditable element in the document (like in the case of the testcase), NotifyEditingStateChanged is not called at all...
Assignee | ||
Comment 2•13 years ago
|
||
Comment 3•13 years ago
|
||
Try run for f0b477074232 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=f0b477074232 Results (out of 193 total builds): success: 173 warnings: 10 failure: 10 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-f0b477074232
Updated•13 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Comment 4•13 years ago
|
||
Comment on attachment 567364 [details] [diff] [review] Patch (v1) Probably fine; can you get peterv to give this a once-over, though?
Attachment #567364 -
Flags: review?(peterv)
Attachment #567364 -
Flags: review?(bzbarsky)
Attachment #567364 -
Flags: review+
Assignee | ||
Comment 5•13 years ago
|
||
Comment on attachment 567364 [details] [diff] [review] Patch (v1) Sure.
Comment 6•13 years ago
|
||
Isn't the problem that NotifyEditableStateChange should call |child->IsEditable()| instead of |child->HasFlag(NODE_IS_EDITABLE)|? IIRC we tried to avoid walking the whole document when designMode was toggled, and so IntrinsicState() checks both the node and its document before returning NS_EVENT_STATE_MOZ_READONLY. If we want to change that (walk the whole document when designMode is toggled) we should go the whole way and make IntrinsicState() just check the node. There might be more changes needed though.
Comment 7•13 years ago
|
||
We store the state in the node directly now, so if the state of all the nodes changed we need to walk the whole document.
Assignee | ||
Comment 8•13 years ago
|
||
Yes, and this is where the bug is stemming from.
Comment 9•13 years ago
|
||
We're still only setting the NODE_IS_EDITABLE flag on non-document nodes for contentEditable, designMode doesn't affect that. So why do we need to drop the |child->HasFlag(NODE_IS_EDITABLE) != aEditable| check instead of making it work for designMode?
Comment 10•13 years ago
|
||
All the testcases in the patch seem to pass with just this change: - if (child->HasFlag(NODE_IS_EDITABLE) != aEditable && - child->IsElement()) { + if (child->IsEditable() != aEditable && child->IsElement()) {
Updated•13 years ago
|
Attachment #567364 -
Flags: review?(peterv) → review+
Assignee | ||
Comment 11•13 years ago
|
||
(In reply to Peter Van der Beken [:peterv] from comment #10) > All the testcases in the patch seem to pass with just this change: > > - if (child->HasFlag(NODE_IS_EDITABLE) != aEditable && > - child->IsElement()) { > + if (child->IsEditable() != aEditable && child->IsElement()) { That would break the test case for bug 672709.
Assignee | ||
Comment 12•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7324c75b47ca
Flags: in-testsuite+
Target Milestone: --- → mozilla10
Comment 13•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7324c75b47ca
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•