Closed
Bug 735059
Opened 12 years ago
Closed 12 years ago
Second and third execCommand() parameters should be optional
Categories
(Core :: DOM: Editor, defect)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: ayg, Assigned: ayg)
Details
(Keywords: dev-doc-needed)
Attachments
(1 file)
3.09 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
It's nice to be able to do execCommand("bold") instead of execCommand("bold", false, ""). All the other browsers allow it, and the spec says it should be allowed too.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → ayg
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•12 years ago
|
||
This bug has actually annoyed me for months.
Attachment #605156 -
Flags: review?(ehsan)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland]
Comment 2•12 years ago
|
||
Comment on attachment 605156 [details] [diff] [review] Patch v1 Review of attachment 605156 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the comment below. ::: editor/libeditor/html/tests/test_bug735059.html @@ +12,5 @@ > +/** Test for Bug 735059 **/ > + > +// Value defaults to the empty string, which evaluates to true, so this > +// disables CSS styling > +document.execCommand("usecss"); If we ever regress this, this call will throw, and the test will finish successfully with a message saying "no actual checks performed" or something similar to that. The correct way to test is to wrap the entire js in a try/catch and explicitly fail the test in the catch block.
Attachment #605156 -
Flags: review?(ehsan) → review+
Updated•12 years ago
|
Whiteboard: [autoland] → [autoland-try]
Updated•12 years ago
|
Whiteboard: [autoland-try] → [autoland-in-queue]
Comment 3•12 years ago
|
||
Autoland Patchset: Patches: 605156 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=cb573ba079ca Try run started, revision cb573ba079ca. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=cb573ba079ca
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #2) > If we ever regress this, this call will throw, and the test will finish > successfully with a message saying "no actual checks performed" or something > similar to that. The correct way to test is to wrap the entire js in a > try/catch and explicitly fail the test in the catch block. I've confirmed that an uncaught exception will actually cause a failure. See bug 365929. If I revert the IDL patch but leave the test, the test fails with message """ an unexpected uncaught JS exception reported through window.onerror - uncaught exception: [Exception... "Not enough arguments [nsIDOMHTMLDocument.execCommand]" nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)" location: "JS frame :: http://mochi.test:8888/tests/editor/libeditor/html/tests/test_bug735059.html?autorun=1&closeWhenDone=1&logFile=%2Fmnt%2Fextra%2Fcheckouts%2Fmozilla-central%2Fobjdir-ff-release%2Fmochitest-plain.log&fileLevel=INFO&consoleLevel=INFO&failureFile=/mnt/extra/checkouts/mozilla-central/objdir-ff-release/_tests/testing/mochitest/makefailures.json :: <TOP_LEVEL> :: line 16" data: no] at :0 """ So no changes needed to the patch, per instructions on IRC.
Keywords: dev-doc-needed
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland-in-queue] → [autoland-try:-u all]
Updated•12 years ago
|
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]
Comment 5•12 years ago
|
||
Autoland Patchset: Patches: 605156 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=975dd82afb07 Try run started, revision 975dd82afb07. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=975dd82afb07
Comment 6•12 years ago
|
||
Indeed, yay for window.onerror
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Whiteboard: [autoland-in-queue]
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5217bbf42aa7
Keywords: checkin-needed
Target Milestone: --- → mozilla14
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5217bbf42aa7
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•