All users were logged out of Bugzilla on October 13th, 2018

code cleanup: use nsINavHistoryService::TRANSITION_EMBED instead of 4

RESOLVED WORKSFORME

Status

()

--
minor
RESOLVED WORKSFORME
11 years ago
9 years ago

People

(Reporter: moco, Assigned: mak)

Tracking

Trunk
Firefox 3.1b3
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 obsolete attachments)

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.
(Assignee)

Comment 1

11 years ago
Created attachment 296132 [details] [diff] [review]
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!
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Attachment #296132 - Flags: review?(sspitzer)
(Assignee)

Comment 2

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

still an issue in SQL fragment, coming with an updated patch
Attachment #296132 - Attachment is obsolete: true
Attachment #296132 - Flags: review?(sspitzer)
(Assignee)

Comment 3

11 years ago
Created attachment 296137 [details] [diff] [review]
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
Attachment #296137 - Flags: review?(sspitzer)
(Assignee)

Comment 4

11 years ago
oops, no, that was not a bug, i use to mark alias with AS :D sorry for confusion.. however that alias is confusing...
marco, can I convince you to hold off on this until after the patch for bug #394038 lands?
(Assignee)

Comment 6

11 years ago
yes, no problem, i was about telling you the same, this can be easily un-bitrotted after
> 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

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

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

over to dietrich for eventual review.
Attachment #296137 - Flags: review?(moco) → review?(dietrich)
(Assignee)

Comment 10

11 years ago
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
Attachment #296137 - Flags: review?(dietrich)
(Assignee)

Comment 11

10 years ago
fsync patch has fixed a bunch of these, still some has to be cleaned up.
Severity: normal → minor
Whiteboard: [needs new patch]
(Assignee)

Comment 12

10 years ago
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
Attachment #296137 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WORKSFORME
(Assignee)

Updated

10 years ago
Whiteboard: [needs new patch]
Target Milestone: --- → Firefox 3.1b3
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.