Closed
Bug 727851
Opened 13 years ago
Closed 13 years ago
Use getSpecialFolderString() in folderWidgets.xml._setCssSelectors
Categories
(Thunderbird :: Folder and Message Lists, defect)
Thunderbird
Folder and Message Lists
Tracking
(Not tracked)
VERIFIED
FIXED
Thunderbird 13.0
People
(Reporter: aceman, Assigned: aceman)
Details
Attachments
(1 file)
(deleted),
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
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().
Attachment #597967 -
Flags: review?(dbienvenu)
Comment 2•13 years ago
|
||
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.
Attachment #597967 -
Flags: review?(dbienvenu) → review+
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.
This bug is just code-deduplication, no behaviour change intended.
Comment 5•13 years ago
|
||
(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.
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.
So what are your results? What about the trybuild?
This patch alone should not intentionally hide the error.
Comment 8•13 years ago
|
||
(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 10•13 years ago
|
||
(In reply to :aceman from comment #9)
> So does the r+ hold?
sure, it's good to remove the code duplication, thx.
Keywords: checkin-needed
Comment 11•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
You need to log in
before you can comment on or make changes to this bug.
Description
•