Closed
Bug 738366
Opened 12 years ago
Closed 12 years ago
Default styleWithCSS to false
Categories
(Core :: DOM: Editor, enhancement)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: ayg, Assigned: ayg)
References
(Depends on 1 open bug)
Details
(Keywords: dev-doc-needed)
Attachments
(1 file, 2 obsolete files)
8.02 KB,
patch
|
ayg
:
review+
|
Details | Diff | Splinter Review |
All other browsers output stuff like b/strong by default, and that's what the spec says. Gecko should too. Authors currently have to work around this by running document.execCommand("useCSS", false, true) all the time, and they shouldn't have to.
Assignee | ||
Comment 1•12 years ago
|
||
Comment 2•12 years ago
|
||
I'm fine with taking this as long as we track this bug for Firefox 14. This is going to break web compat at some place, so we should track this and make sure that any regressions reported from this are going to be tracked properly.
tracking-firefox14:
--- → +
Comment 3•12 years ago
|
||
Comment on attachment 608410 [details] [diff] [review] Patch v1 Can you also add code to test_bug738366.html to make sure that issuing the stylewithcss command toggles the behavior? r=me with that.
Attachment #608410 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 4•12 years ago
|
||
Okay.
Attachment #608410 -
Attachment is obsolete: true
Attachment #608450 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland-try:-u all]
Updated•12 years ago
|
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]
Comment 5•12 years ago
|
||
Autoland Patchset: Patches: 608450 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=9b8810d40bea Try run started, revision 9b8810d40bea. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=9b8810d40bea
Comment 6•12 years ago
|
||
Try run for 9b8810d40bea is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=9b8810d40bea Results (out of 219 total builds): success: 181 warnings: 24 failure: 14 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-9b8810d40bea
Updated•12 years ago
|
Whiteboard: [autoland-in-queue]
Assignee | ||
Comment 7•12 years ago
|
||
Not sure if this will actually fix the unexpected passes, because I couldn't reproduce them locally. Let's see.
Attachment #608450 -
Attachment is obsolete: true
Attachment #608737 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland-try:-u crashtest -b d]
Updated•12 years ago
|
Whiteboard: [autoland-try:-u crashtest -b d] → [autoland-in-queue]
Comment 8•12 years ago
|
||
Autoland Patchset: Patches: 608737 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=595b6c572e72 Try run started, revision 595b6c572e72. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=595b6c572e72
Comment 9•12 years ago
|
||
Try run for 595b6c572e72 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=595b6c572e72 Results (out of 13 total builds): success: 6 warnings: 7 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-595b6c572e72
Updated•12 years ago
|
Whiteboard: [autoland-in-queue]
Assignee | ||
Comment 10•12 years ago
|
||
Ehsan, I looked at the new assertion but didn't understand it. The interesting part of the stack is: ^G###!!! ASSERTION: null node passed to GetBlockNodeParent(): 'Not Reached', file /mnt/extra/checkouts/mozilla-central/editor/libeditor/html/nsHTMLEditor.cpp, line 931 nsHTMLEditor::GetBlockNodeParent(nsIDOMNode*) (/mnt/extra/checkouts/mozilla-central/editor/libeditor/html/nsHTMLEditor.cpp:932) nsHTMLEditRules::WillDeleteSelection(nsISelection*, short, bool*, bool*) (/mnt/extra/checkouts/mozilla-central/editor/libeditor/html/nsHTMLEditRules.cpp:2187) nsHTMLEditRules::WillDoAction(nsISelection*, nsRulesInfo*, bool*, bool*) (/mnt/extra/checkouts/mozilla-central/editor/libeditor/html/nsHTMLEditRules.cpp:617) nsPlaintextEditor::DeleteSelection(short) (/mnt/extra/checkouts/mozilla-central/editor/libeditor/text/nsPlaintextEditor.cpp:789) nsDeleteCommand::DoCommand(char const*, nsISupports*) (/mnt/extra/checkouts/mozilla-central/editor/libeditor/base/nsEditorCommands.cpp:630) nsControllerCommandTable::DoCommand(char const*, nsISupports*) (/mnt/extra/checkouts/mozilla-central/embedding/components/commandhandler/src/nsControllerCommandTable.cpp:191) nsBaseCommandController::DoCommand(char const*) (/mnt/extra/checkouts/mozilla-central/embedding/components/commandhandler/src/nsBaseCommandController.cpp:169) nsCommandManager::DoCommand(char const*, nsICommandParams*, nsIDOMWindow*) (/mnt/extra/checkouts/mozilla-central/embedding/components/commandhandler/src/nsCommandManager.cpp:273) nsHTMLDocument::ExecCommand(nsAString_internal const&, bool, nsAString_internal const&, bool*) (/mnt/extra/checkouts/mozilla-central/content/html/document/src/nsHTMLDocument.cpp:3100) This is probably the same as the following line: WARNING: NS_ENSURE_TRUE(leftParent && rightParent) failed: file /mnt/extra/checkouts/mozilla-central/editor/libeditor/html/nsHTMLEditRules.cpp, line 2194 The chance that this was actually caused by my change seems to be approximately zero, so is it okay if I just file a followup bug and increase the expected number of assertions to two for this test?
Comment 11•12 years ago
|
||
Hmm, this is weird. nsHTMLEditRules::WillDeleteSelection should not call GetBlockNodeParent, right? So how does this happen?
Comment 12•12 years ago
|
||
If the reason for the assertion is not obvious to you, I'd be fine with you filing a follow-up bug for that.
Assignee | ||
Comment 13•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/aaba08c21296 Bug 671153 covers the same exception, so I didn't file a separate followup.
Comment 14•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/aaba08c21296
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 15•12 years ago
|
||
Should probably get mentioned in the docs, since this is considered a compatibility risk.
Keywords: dev-doc-needed
Updated•12 years ago
|
status-firefox14:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•