727851 - Use getSpecialFolderString() in folderWidgets.xml._setCssSelectors

Status: VERIFIED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[:aceman]

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]

Attachment: patch

Comment 2

[David :Bienvenu]

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]

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]

This bug is just code-deduplication, no behaviour change intended.

Comment 5

[David :Bienvenu]

(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]

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]

So what are your results? What about the trybuild?
This patch alone should not intentionally hide the error.

Comment 8

[David :Bienvenu]

(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]

So does the r+ hold?

Comment 10

[David :Bienvenu]

(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)]

Checked in: http://hg.mozilla.org/comm-central/rev/d182f15b6b63