Closed Bug 367799 Opened 16 years ago Closed 15 years ago

nsNavHistory duplicates "effective TLD" functionality

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 beta4

People

(Reporter: hello, Assigned: dwitte)

References

()

Details

Attachments

(2 files)

nsNavHistory.cpp has some functions for finding out the effective TLD of a URL (see GetTLDType and friends). This is now available as a service, and we should switch to using that instead.

We might want to use the helpers proposed in bug #367446 if/when they go in.
Depends on: 368989
No longer depends on: 367446
I'm not likely to be able to work on these bugs anytime soon.
Assignee: thunder → nobody
-> me. always nice to see more etld consumers!
Assignee: nobody → dwitte
OS: Mac OS X → All
Hardware: PC → All
Target Milestone: --- → Firefox 3 M10
Attached patch patch v1Splinter Review
switches over to the etldservice and removes unnecessary code. (note that this patch depends on bug 402008 and bug 402013, which affect the normalization semantics here.)

dan, would you mind reviewing this? (if not, do you know who'd be appropriate?) thanks!
Attachment #287814 - Flags: review?(thunder)
Depends on: 402008, 402013
this is ready to land once it gets review (the two bugs that block this have been fixed).
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Comment on attachment 287814 [details] [diff] [review]
patch v1

Looks OK, and tested by hacking my build to GROUP_BY_DOMAIN in the sidebar.  It is hot.
Attachment #287814 - Flags: review?(thunder) → review+
Dietrich, is a second r+ needed?

You might want to take a look anyway, it's been a few months since I've really been in nsNavHistory.cpp.
Comment on attachment 287814 [details] [diff] [review]
patch v1

a=mconnor on behalf of drivers.  yay for no more duplication
Attachment #287814 - Flags: approval1.9+
dwitte - plan on landing this when the tree opens?
Target Milestone: Firefox 3 beta3 → Firefox 3 beta4
yep, for b4
FWIW, I've been using this patch + a patch to enable GROUP_BY_DOMAIN in the sidebar in my personal builds.  It's very nice.
sdwilsh, dietrich - it looks like the GROUP_BY_DOMAIN functionality has been sitting around, but isn't used anywhere by default yet (comment 11) - is this something we want to enable for firefox 3? it sounds pretty cool.
checked in, with above tweak to init the eTLD and IDN services lazily, and thus avoid Ts penalty. (since this feature isn't even really used, yet...)

fixed!
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
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.