Closed
Bug 935958
Opened 11 years ago
Closed 10 years ago
"unsupported sort key: 17" output from Seamonkey
Categories
(SeaMonkey :: General, defect)
SeaMonkey
General
Tracking
(seamonkey2.24 affected, seamonkey2.25 fixed, seamonkey2.26 fixed, seamonkey2.27 fixed)
RESOLVED
FIXED
seamonkey2.27
People
(Reporter: ishikawa, Assigned: ishikawa)
References
Details
Attachments
(1 file)
1.05 KB,
patch
|
iannbugzilla
:
review+
iannbugzilla
:
superreview+
iannbugzilla
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #895085 +++ unsupported sort key: 17 During the run of DEBUG BUILD of thunderbird, I get an output like the following 34 times. unsupported sort key: 17 I tracked down to this output to the following code in ConvertSortTypeToColumnID() in https://mxr.mozilla.org/comm-central/source/mail/base/content/commandglue.js See line 261 187 function ConvertSortTypeToColumnID(sortKey) 188 { 189 var columnID; 190 191 // Hack to turn this into an integer, if it was a string. 192 // It would be a string if it came from localStore.rdf 193 sortKey = sortKey - 0; 194 195 switch (sortKey) { 196 case nsMsgViewSortType.byDate: 197 columnID = "dateCol"; 198 break; 199 case nsMsgViewSortType.byReceived: 200 columnID = "receivedCol"; 201 break; ... 244 case nsMsgViewSortType.byAttachments: 245 columnID = "attachmentCol"; 246 break; 247 case nsMsgViewSortType.byCustom: 248 249 //TODO: either change try() catch to if (property exists) or restore the getColumnHandler() check 250 try //getColumnHandler throws an errror when the ID is not handled 251 { 252 columnID = gDBView.curCustomColumn; 253 } 254 catch (err) { //error - means no handler 255 dump("ConvertSortTypeToColumnID: custom sort key but no handler for column '" + columnID + "'\n"); 256 columnID = "dateCol"; 257 } 258 259 break; 260 default: 261 dump("unsupported sort key: " + sortKey + "\n"); <------ HERE 262 columnID = "dateCol"; 263 break; 264 } 265 return columnID; 266 } Basically, the code is printing "unsupported sort key:" message since the key is not handled in the switch statement. But then what is this "17", then? I looked for the defines for the values used in case labels. They are defined in the following file. https://mxr.mozilla.org/comm-central/source/mailnews/base/public/nsIMsgDBView.idl#60 60 interface nsMsgViewSortType 61 { 62 const nsMsgViewSortTypeValue byNone = 0x11; /* not sorted */ 63 const nsMsgViewSortTypeValue byDate = 0x12; 64 const nsMsgViewSortTypeValue bySubject = 0x13; ... 79 const nsMsgViewSortTypeValue byCustom = 0x22; 80 const nsMsgViewSortTypeValue byReceived = 0x23; 81 }; 17 is 0x11, the value of |byNone|. So the code was complaining that a folder (or is it a newsgroup?) is "not sorted" somehow. I don't know the life cycle of a folder, but somehow such "not sorted" folder is created during the test run of "make mozill" of thunderbird. (And for that matter, I see this unsupported key: 17 message during my use of TB at the office. Usually, I see it when I do a global search across folders. So maybe one or more folders in my setup have this "not sorted" property somehow.) When a folder is "not sorted", the code sets the sortKey |columnID| to "dateCol", which seems a reasonable default. So maybe we don't have to print out the warning in the case of |byNone|. A fix is suggested as attached. Since the code is very old(?) and I could not find meaningful hg blame output, I am requesting review from Daivd (mozilla@davidbienvenu.org) TIA Actually there is similar code in SeaMonkey as well. I searched the string using the following command and got the output as below. find . \( -name ".hg" -prune \) -o \( -type f -print \) | xargs egrep "unsupported sort key" ./mail/base/content/commandglue.js: dump("unsupported sort key: " + sortKey + "\n"); ./suite/mailnews/commandglue.js: dump("unsupported sort key: " + sortKey + "\n"); The same fix is needed for both TB and SeaMonkey. So I am attaching the patch for both of them. PPS: I noticed during the run of "make mozmill" test suite of DEBUG BUILD of thunderbird. PPPS: Patch was checked in TB tryserver submission. (No new problems were found other than the permanent orange type of errors.) https://tbpl.mozilla.org/?rev=13f6001f5c01&tree=Thunderbird-Try PPPPS: If a folder should not have "not sorted" property, then maybe this warning output is useful. But as far as I know, my office TB usage printed out this warning for a long time (at least a few years). So it seems OK to have "not sorted" property for folders. ---
Assignee | ||
Comment 2•11 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #1) > Doesn't have r+ Sorry, I should have cloned the bugzilla before I gave checkin-needed to Bug 895085. I am duplicating this bug since the original bugzilla entry tried to address the same issue for both - TB, and - Seamonkey. Exactly the same JS file was affected by the original bugzilla. The patch for TB portion was approved, but the reviewer wanted to have a reviewer for suite portion, and so I split the bugzilla (I should have split it before I gave the original checkin-needed keyword. I didn't realize this was duped as well.) The original takes care of TB issue only, and this bugzilla entry takes care of Seamonkey issue only. I have uploaded the patch, which is basically the same patch to be applied TB, here for Seamonkey. I will find out who the reviewer should be. TIA
Assignee | ||
Updated•11 years ago
|
Summary: "unsupported sort key: 17" output from Thunderbird → "unsupported sort key: 17" output from Seamonkey
Assignee | ||
Comment 3•11 years ago
|
||
> Exactly the same JS file was affected by the original bugzilla.
I should have said
A file used in TB has exactly the same content as a file, under different name,
used by Seamonkey and both files ought to be fixed.
TIA
Attachment #828622 -
Flags: review?(neil)
Comment 4•11 years ago
|
||
Comment on attachment 828622 [details] [diff] [review] Fix to suppress printing of "unsupported key: 17" for "not sorted" folder. (same patch has been approved for TB already). Adding r? IanN (or whoever gets to review this first) moa? Mnyromyr.
Attachment #828622 -
Flags: superreview?(mnyromyr)
Attachment #828622 -
Flags: review?(iann_bugzilla)
Comment 5•11 years ago
|
||
Comment on attachment 828622 [details] [diff] [review] Fix to suppress printing of "unsupported key: 17" for "not sorted" folder. (same patch has been approved for TB already). I'm not a fan of this approach, although I see the custom column handler code already defaults to dateCol when the sort type is custom but no custom sort key was defined, so I don't know what to say here. (Then again, we should never see a sort key of byNone in SeaMonkey as we don't have that particular search.)
Attachment #828622 -
Flags: review?(neil)
Comment 6•11 years ago
|
||
(In reply to ISHIKAWA, Chiaki from comment #2) […] > I will find out who the reviewer should be. > > TIA In case you ever come back to fix other bugs in SeaMonkey (one can always hope ;-) ), I recommend that you bookmark the following two pages: the list of reviewers by module at http://www.seamonkey-project.org/dev/project-areas and the policies about review, superreview, et al. at http://www.seamonkey-project.org/dev/review-and-flags
Comment on attachment 828622 [details] [diff] [review] Fix to suppress printing of "unsupported key: 17" for "not sorted" folder. (same patch has been approved for TB already). r/sr=me [Triage Comment] a=me for c-a so it lands on the same branches that it did for TB assuming the uplift has happened.
Attachment #828622 -
Flags: superreview?(mnyromyr)
Attachment #828622 -
Flags: superreview+
Attachment #828622 -
Flags: review?(iann_bugzilla)
Attachment #828622 -
Flags: review+
Attachment #828622 -
Flags: approval-comm-aurora+
Comment 8•10 years ago
|
||
Hi ISHIKAWA-san. Did you ever check this in? I don't see this in comm-central, and I did not see a [check-in needed] keyword.
Flags: needinfo?(ishikawa)
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Philip Chee from comment #8) > Hi ISHIKAWA-san. Did you ever check this in? I don't see this in > comm-central, and I did not see a [check-in needed] keyword. Oh, my. I have obviously forgotten to put the [check-in needed] keyword! I am just about to do it. Sorry for my oversight, and thank you for noticing this! TIA
Flags: needinfo?(ishikawa)
Keywords: checkin-needed
Comment 10•10 years ago
|
||
Pushed: https://hg.mozilla.org/comm-central/rev/8cff2e56d482 https://hg.mozilla.org/releases/comm-aurora/rev/301c46466eac https://hg.mozilla.org/releases/comm-beta/rev/632d073bfcc5 Missed the uplift so this patch will only make it to SeaMonkey 2.25. Sorry about that.
Status: NEW → RESOLVED
Closed: 10 years ago
status-seamonkey2.24:
--- → affected
status-seamonkey2.25:
--- → fixed
status-seamonkey2.26:
--- → fixed
status-seamonkey2.27:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.27
Updated•10 years ago
|
Keywords: checkin-needed
You need to log in
before you can comment on or make changes to this bug.
Description
•