Closed Bug 836094 Opened 7 years ago Closed 7 years ago

Remove uses of RESULT_TYPE_FULL_VISIT and RESULT_TYPE_DYNAMIC_CONTAINER from comm-central, now that bug 835543 removed those constants

Categories

(SeaMonkey :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.18

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file)

nsNavHistoryResultNode::RESULT_TYPE_FULL_VISIT has gone away (on m-i), in bug 835543.

But, NeilAway points out in IRC that we have a line of code that checks for it in comm-central:
  https://mxr.mozilla.org/comm-central/source/suite/common/places/controller.js#468

(I'm pretty sure that's the only usage in comm-central outside of code that's taken directly from m-c.)

We should just delete that line.  Ideally this should happen around when bug 835543's patches are merged over to comm-central, but it doesn't really matter, since (IIUC) we never return nodes of type RESULT_TYPE_FULL_VISIT anyway, so we should behave identically regardless of whether that line is there.
Product: Thunderbird → SeaMonkey
Version: unspecified → Trunk
Actually, we need to fix this before bug 835543's patches are merged over, because they ended up commenting out the constant RESULT_TYPE_FULL_VISIT, so any code using it won't compile anymore.

(I also commented out RESULT_TYPE_DYNAMIC_CONTAINER, and it turns out comm-central has some stale code using that, which won't compile anymore, too:
  https://mxr.mozilla.org/comm-central/source/suite/common/places/controller.js#456
)
Summary: Remove usage of RESULT_TYPE_FULL_VISIT in comm-central, now that bug 835543 is fixed → Remove uses of RESULT_TYPE_FULL_VISIT and RESULT_TYPE_DYNAMIC_CONTAINER from comm-central, now that bug 835543 removed those constants
(er, s/won't compile/will throw an exception or something/, since this is JS)
Attached patch fix v1Splinter Review
mak, it should be safe to just drop these from this switch statement, because we'll never have nodes of this type, correct?
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #707879 - Flags: review?
Attachment #707879 - Flags: feedback?(mak77)
Attachment #707879 - Flags: review? → review?(neil)
Summary: Remove uses of RESULT_TYPE_FULL_VISIT and RESULT_TYPE_DYNAMIC_CONTAINER from comm-central, now that bug 835543 removed those constants → Remove uses of RESULT_TYPE_FULL_VISIT and RESULT_TYPE_DYNAMIC_CONTAINER from comm-central, now that bug 835543 removed those constants
Comment on attachment 707879 [details] [diff] [review]
fix v1

Thanks for the patch, and r=me if you remove the associated comment on line 413, as per bug 700296.
Attachment #707879 - Flags: review?(neil) → review+
Ah, good catch!

I think mak might be asleep, and I'm 99% sure this is fine (especially w/ the analogous change from bug 700296), so I went ahead and pushed:
  https://hg.mozilla.org/comm-central/rev/f91edf237956

(If mak happens to have any 'omg that's wrong!' feedback, we can always back out and/or use followup bugs.)
Comment on attachment 707879 [details] [diff] [review]
fix v1

OMG, that's correct!

actually I thought the places controller in Seamonkey was more in sync with Firefox...
Attachment #707879 - Flags: feedback?(mak77) → feedback+
Bug looks fixed to me.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.18
Yup, thanks. (Sorry, I should have closed it in comment 5)
You need to log in before you can comment on or make changes to this bug.