410884 - code cleanup: use nsINavHistoryService::TRANSITION_EMBED instead of 4

Status: RESOLVED WORKSFORME

(Firefox :: Bookmarks & History, defect)

Description

[(not reading, please use [e-mail] instead)]

code cleanup:  use  nsINavHistoryService::TRANSITION_EMBED instead of 4

this was part of my patch for bug #394038, but I've taken it out to avoid conflicts with Marco's fix for bug #392399 

In our code, we currently do "visit_type NOT IN (0,4)"

We should not hard code 4, but I think we should be using nsINavHistoryService::TRANSITION_EMBED instead.

this is a minor issue.

Comment 1

[Marco Bonardo [:mak]]

Attachment: cleanup + query bug fix

i've changed everything to not use fixed values (also in bookmarks) and use IN

what about adding a TRANSITION_UNDEFINED for 0 ?

while fixing this i found a bug in: 
"SELECT MIN(visit_date) id FROM moz_historyvisits WHERE visit_type NOT IN(0,4)"
this is a real bug, not only cleanup (fixed in patch) since the query syntax is wrong!

Comment 2

[Marco Bonardo [:mak]]

Comment on attachment 296132 [details] [diff] [review]
cleanup + query bug fix

still an issue in SQL fragment, coming with an updated patch

Comment 3

[Marco Bonardo [:mak]]

Attachment: cleanup + query bug fix

i clearly cannot put a macro in a macro... i'm leaving sql_str_fragment with hardcoded 4, added a comment to point out what is 4 there

Comment 4

[Marco Bonardo [:mak]]

oops, no, that was not a bug, i use to mark alias with AS :D sorry for confusion.. however that alias is confusing...

Comment 5

[(not reading, please use [e-mail] instead)]

marco, can I convince you to hold off on this until after the patch for bug #394038 lands?

Comment 6

[Marco Bonardo [:mak]]

yes, no problem, i was about telling you the same, this can be easily un-bitrotted after

Comment 7

[(not reading, please use [e-mail] instead)]

> yes, no problem

Thanks, Marco.  That will make Dietrich's life easier.

Note, in bug #394038 (or a spin off) we will probably be changing this to be NOT IN(0,4,7) for TRANSITION_DOWNLOAD.

Comment 8

[Seth Spitzer]

> Note, in bug #394038 (or a spin off)

see bug #412217

Comment 9

[(not reading, please use [e-mail] instead)]

Comment on attachment 296137 [details] [diff] [review]
cleanup + query bug fix

over to dietrich for eventual review.

Comment 10

[Marco Bonardo [:mak]]

Comment on attachment 296137 [details] [diff] [review]
cleanup + query bug fix

thank you seth :) 
i'm clearing review for now, should be done after frecency patch and unbitrotting

Comment 11

[Marco Bonardo [:mak]]

fsync patch has fixed a bunch of these, still some has to be cleaned up.

Comment 12

[Marco Bonardo [:mak]]

Comment on attachment 296137 [details] [diff] [review]
cleanup + query bug fix

the only remaining references to 0,4,7 transitions left are in comments or macros, so we should be done here

Comment 13

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv