Closed
Bug 572618
Opened 14 years ago
Closed 14 years ago
Make debugging the editor easier
Categories
(Core :: DOM: Editor, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b1
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(4 files, 5 obsolete files)
660.89 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
376.79 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
248.02 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
74.97 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
It's been a long time that I wanted to do this. We can use warning messages to make debugging the editor easier, by switching to using macros like NS_ENSURE_*. This can be done using automatic search and replacement. It won't change any behavior, but would make reading the code easier as an added bonus.
Assignee | ||
Comment 1•14 years ago
|
||
This is the result of this command: find . -type f -name \*.cpp -or -name \*.h | xargs perl -pi -e 's/if\s+\(NS_FAILED\(([^)]+)\)\)\s+return\s+([^;]+)/NS_ENSURE_SUCCESS(\1, \2)/g'
Attachment #451846 -
Flags: review?(roc)
Assignee | ||
Comment 2•14 years ago
|
||
This is the result of this command: find . -type f -name \*.cpp -or -name \*.h | xargs perl -pi -e 's/if\s+\(!([^)]+)\)\s+return\s+([^;]+)/NS_ENSURE_TRUE(\1, \2)/g' with two manual tweaks in nsHTMLDataTransfer.cpp because in two places in that file, the |if| in question had an |else|.
Attachment #451849 -
Flags: review?(roc)
Assignee | ||
Comment 3•14 years ago
|
||
This is the result of this command: find . -type f -name \*.cpp -or -name \*.h | xargs perl -pi -e 'undef $/; s/if\s+\(!([^)]+)\)\s*\n\s*return\s+([^;]+)/NS_ENSURE_TRUE(\1, \2)/gms' with one manual fix in nsHTMLEditor.cpp of this pattern: if (foo) // if (bar) return NS_ERROR_baz;
Attachment #451857 -
Flags: review?(roc)
Assignee | ||
Comment 4•14 years ago
|
||
This is the result of this command: find . -type f -name \*.cpp -or -name \*.h | xargs perl -pi -e 'undef $/; s/if\s+\(NS_FAILED\(([^)]+)\)\)\s*\n\s*return\s+([^;]+)/NS_ENSURE_SUCCESS(\1, \2)/g'
Attachment #451859 -
Flags: review?(roc)
Assignee | ||
Comment 5•14 years ago
|
||
Handle compound conditions correctly. For example, handle constructs such as the following correctly: - if (!foo || !bar) return baz; + NS_ENSURE_TRUE(foo && bar, baz); Interdiff with the previous version of part 2: http://pastebin.mozilla.org/736447
Attachment #451849 -
Attachment is obsolete: true
Attachment #451869 -
Flags: review?(roc)
Attachment #451849 -
Flags: review?(roc)
Assignee | ||
Comment 6•14 years ago
|
||
I had to update these patches when I removed the patch to bug 572513 from my queue. I'll attach the new versions.
Assignee | ||
Comment 7•14 years ago
|
||
Attachment #451846 -
Attachment is obsolete: true
Attachment #452065 -
Flags: review?(roc)
Attachment #451846 -
Flags: review?(roc)
Assignee | ||
Comment 8•14 years ago
|
||
Attachment #451869 -
Attachment is obsolete: true
Attachment #452066 -
Flags: review?(roc)
Attachment #451869 -
Flags: review?(roc)
Assignee | ||
Comment 9•14 years ago
|
||
Attachment #451857 -
Attachment is obsolete: true
Attachment #452068 -
Flags: review?(roc)
Attachment #451857 -
Flags: review?(roc)
Assignee | ||
Comment 10•14 years ago
|
||
Attachment #451859 -
Attachment is obsolete: true
Attachment #452070 -
Flags: review?(roc)
Attachment #451859 -
Flags: review?(roc)
Attachment #452065 -
Flags: review?(roc) → review+
Attachment #452066 -
Flags: review?(roc) → review+
Attachment #452068 -
Flags: review?(roc) → review+
Attachment #452070 -
Flags: review?(roc) → review+
Assignee | ||
Comment 11•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/8d5a112a3091 http://hg.mozilla.org/mozilla-central/rev/223b2b67edbd http://hg.mozilla.org/mozilla-central/rev/bebdf3413522 http://hg.mozilla.org/mozilla-central/rev/45dd34892448
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a6
I backed out one of the changes: http://hg.mozilla.org/mozilla-central/rev/5332eb29d243 because it was causing huge amounts of debug spew, causing some tinderbox builds to fail due to huge log size: WARNING: NS_ENSURE_TRUE(mRedoStack) failed: file e:/builds/moz2_slave/mozilla-central-win32-debug/build/editor/txmgr/src/nsTransactionItem.cpp, line 337
Comment 13•14 years ago
|
||
diff --git a/editor/libeditor/base/nsEditorEventListener.cpp b/editor/libeditor/base/nsEditorEventListener.cpp ... nsCOMPtr<nsIDOMNode> sourceNode; dataTransferNS->GetMozSourceNode(getter_AddRefs(sourceNode)); - if (!sourceNode) - return PR_TRUE; + NS_ENSURE_TRUE(sourceNode, PR_TRUE); This case isn't an error condition. Was this change desired? It causes the warning to dump out continuously from an external location while dragging over a textbox.
Assignee | ||
Comment 14•14 years ago
|
||
(In reply to comment #13) > diff --git a/editor/libeditor/base/nsEditorEventListener.cpp > b/editor/libeditor/base/nsEditorEventListener.cpp > ... > nsCOMPtr<nsIDOMNode> sourceNode; > dataTransferNS->GetMozSourceNode(getter_AddRefs(sourceNode)); > - if (!sourceNode) > - return PR_TRUE; > + NS_ENSURE_TRUE(sourceNode, PR_TRUE); > > This case isn't an error condition. Was this change desired? It causes the > warning to dump out continuously from an external location while dragging over > a textbox. This was a bogus check. I removed it in http://hg.mozilla.org/mozilla-central/rev/5f38f9a55345. Thanks for catching this.
You need to log in
before you can comment on or make changes to this bug.
Description
•