Last Comment Bug 727851 - Use getSpecialFolderString() in folderWidgets.xml._setCssSelectors
: Use getSpecialFolderString() in folderWidgets.xml._setCssSelectors
Status: VERIFIED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Folder and Message Lists (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: Thunderbird 13.0
Assigned To: :aceman
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-16 08:36 PST by :aceman
Modified: 2012-02-23 08:22 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (3.70 KB, patch)
2012-02-16 12:45 PST, :aceman
mozilla: review+
Details | Diff | Splinter Review

Description :aceman 2012-02-16 08:36:49 PST
Noticed in bug 670976 comment 26.

There is duplicated code in folderWidgets.xml._setCssSelectors, which is already defined in /mailnews/base/util/folderUtils.jsm getSpecialFolderString().
Comment 1 :aceman 2012-02-16 12:45:18 PST
Created attachment 597967 [details] [diff] [review]
patch
Comment 2 David :Bienvenu 2012-02-17 11:59:30 PST
Comment on attachment 597967 [details] [diff] [review]
patch

thx, this does seem to fix the aFolder error (that was the intent, right?). I've requested a try server build to make sure mozmill tests still pass, but it looks good to me.
Comment 3 :aceman 2012-02-17 12:10:16 PST
No, this will actually confuse the debugging even more, it should now throw inside getSpecialFolderString, where aFolder.flags is accessed.

Attempt to solve the error is properly separated in bug 670976.
Comment 4 :aceman 2012-02-17 12:14:40 PST
This bug is just code-deduplication, no behaviour change intended.
Comment 5 David :Bienvenu 2012-02-17 15:31:03 PST
(In reply to :aceman from comment #3)
> No, this will actually confuse the debugging even more, it should now throw
> inside getSpecialFolderString, where aFolder.flags is accessed.

that exception seems to get caught and suppressed; I don't see anything on the error console anymore.
Comment 6 :aceman 2012-02-17 15:40:36 PST
If you didn't apply patch from bug 670976 too then it would be strange. The change here should not have the effect of suppressing it. Do you have proper STRs? Maybe exceptions are hidden inside .jsm files?
Just place an alert (or other dump) before the line
'aMenuNode.setAttribute("SpecialFolder", getSpecialFolderString(aFolder));' and you should see aFolder is undefined.
Comment 7 :aceman 2012-02-18 04:57:33 PST
So what are your results? What about the trybuild?
This patch alone should not intentionally hide the error.
Comment 8 David :Bienvenu 2012-02-19 07:50:35 PST
(In reply to :aceman from comment #7)
> So what are your results? What about the trybuild?
> This patch alone should not intentionally hide the error.

turns out my STR's (the run message filter steps) were not reliable, though I did try it a couple times w/ and w/o the patch. So ignore me.
Comment 9 :aceman 2012-02-20 12:22:01 PST
So does the r+ hold?
Comment 10 David :Bienvenu 2012-02-20 12:36:59 PST
(In reply to :aceman from comment #9)
> So does the r+ hold?

sure, it's good to remove the code duplication, thx.
Comment 11 Mark Banner (:standard8, limited time in Dec) 2012-02-21 07:36:14 PST
Checked in: http://hg.mozilla.org/comm-central/rev/d182f15b6b63

Note You need to log in before you can comment on or make changes to this bug.