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

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Daniel Holbert [:dholbert]]

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
)

Comment 2

[Daniel Holbert [:dholbert]]

(er, s/won't compile/will throw an exception or something/, since this is JS)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: fix v1

mak, it should be safe to just drop these from this switch statement, because we'll never have nodes of this type, correct?

Comment 4

[[e-mail]]

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.

Comment 5

[Daniel Holbert [:dholbert]]

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 6

[Marco Bonardo [:mak]]

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...

Comment 7

[Frank Wein [:mcsmurf]]

Bug looks fixed to me.

Comment 8

[Daniel Holbert [:dholbert]]

Yup, thanks. (Sorry, I should have closed it in comment 5)