Expose visit ID in result nodes.

RESOLVED WONTFIX

Status

()

Toolkit
Places
P3
normal
RESOLVED WONTFIX
12 years ago
5 years ago

People

(Reporter: Brett Wilson, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.2 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
 
(Reporter)

Updated

12 years ago
Priority: -- → P2
(Reporter)

Updated

12 years ago
Blocks: 318653
(Reporter)

Updated

12 years ago
Depends on: 323492
Priority: P2 → P3
Looks like this is fixed. Correct me if I'm wrong. 
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Reporter)

Comment 2

12 years ago
Not fixed. The interface is there, but RESULTS_AS_FULL_VISIT is not implemented yet. Should be easy, though.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: --- → Firefox 2 beta1
(Reporter)

Updated

11 years ago
Priority: P3 → P4
Target Milestone: Firefox 2 beta1 → Firefox 3
(Reporter)

Updated

11 years ago
Assignee: brettw → nobody
Status: REOPENED → NEW
Duplicate of this bug: 392281

Comment 4

10 years ago
Is this still targeted for Firefox 3?
Drivers: We should consider blocking on this, so that extensions can utilize it for walking visit trails.
Flags: blocking-firefox3?
Priority: P4 → P3

Updated

10 years ago
Blocks: 392844

Comment 6

10 years ago
For example, https://www.squarefree.com/extensions/high/ needs this API in order to be ported to trunk.  It has 3000 daily active users according to preview.addons.mozilla.org.
(Reporter)

Comment 7

10 years ago
Note that the visit trails are fairly poor quality. There are a number of situations where the trail will get "lost" and so an extension author wanting to count on this might be disappointed. However, it should be at least as reliable as the former "referrer" field in history, so maybe it's good enough for users like this.

Not that it's also somewhat non-trivial to use these IDs since you can't query for them, so you would have to suck down all visits for a period of history and search for them manually. It should be easy and low-risk to take if somebody wants, but I wouldn't block on it.

Comment 8

10 years ago
> Note that the visit trails are fairly poor quality. There are a number of
> situations where the trail will get "lost" and so an extension author wanting
> to count on this might be disappointed.

Can you elaborate?  I might want to document this as part of the extension's web page.

> Not that it's also somewhat non-trivial to use these IDs since you can't query
> for them, so you would have to suck down all visits for a period of history and
> search for them manually.

My extension wants to query by URI, so I don't think I'd have that problem.
(Reporter)

Comment 9

10 years ago
I recall (a bit misty now) that it keys off of the HTTP referrer, so any time you don't have that like http->https and certain JavaScript stuff.

We used to have lines in the history view that separated contiguous trails, and there would often be superfluous lines. I think in addition to the issues mentioned above, there may also be other problems. Nobody has used this information for anything before, so it's difficult to tell.

This will not block the final release of Firefox 3.
Flags: blocking-firefox3? → blocking-firefox3-

Comment 11

10 years ago
Brett:
> I recall (a bit misty now) that it keys off of the HTTP referrer, so any time
> you don't have that like http->https and certain JavaScript stuff.

Why not improve this to do it based on actual page request source to get better trails? What's required to do this (some rough pointers, please)?

Mike:
> This will not block the final release of Firefox 3.

Is there any chance someone will implement the RESULTS_AS_FULL_VISIT before FF3 release? (If someone does agree to do this soon, will it be approved in time?)

The fact that the trails are of poor quality for now can possibly be fixed in the future to improve user experience for those extensions that use the new places API - as long as the API is fully implemented *now* to reveal referringId (and the other stuff in RESULTS_AS_FULL_VISIT).

Without this, the whole move to Places will have little impact on extension developers.
(Reporter)

Comment 12

10 years ago
> Why not improve this to do it based on actual page request source to get better
> trails?

I don't know how to do this. It would require changing some scary internals like the docshell.
Blocks: 437245
Target Milestone: Firefox 3 → Firefox 3.1

Comment 13

9 years ago
The "How'd I Get Here?" extension depends on this (it's a button you click on to go to the first web page from which you go to the page you're on). Also required would be https://bugzilla.mozilla.org/show_bug.cgi?id=392844

https://www.squarefree.com/extensions/high/
https://addons.mozilla.org/en-US/firefox/addon/1196

Comment 14

9 years ago
I do not think the quality of results is as important as just getting SOME sort of trail information.

I find the "How'd I Get Here" is one of my most frequently used extensions. It's invaluable when opening multiple links in new tabs, as you can go to the tab and still get back to where you came from in that new tab. Something you cannot do otherwise (the back button is grayed out on new pages opened in tabs). 

And not only that, but it works from pages opened through bookmarks too. Why did I bookmark this page? From where did I find it? Click on the "How'd I Get Here" and you have your answer.

Is there any way to get this API implemented? I'd take low-quality trails any day, compared to no trails.

Comment 15

9 years ago
You don't need an extension for that use case.  A bookmarklet suffices:

javascript:void(location=document.referrer);

Comment 16

9 years ago
Great, but it doesn't work with bookmarks.

Comment 17

9 years ago
Another addon work-around that might partially satisfy Casper's case is Tab History https://addons.mozilla.org/en-US/firefox/addon/1859

Comment 18

9 years ago
Is this still targeted for 3.1? If there's no-one actively working on it, and it is the sort of thing that an external contributor could take a crack at, would it be possible to get a quick outline of what needs to change to fully support RESULTS_AS_FULL_VISIT?
Hi David,

This bug does not currently have an owner, so you're welcome to it. It'd be great to get this in 3.1.

* here's the interface: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/public/nsINavHistoryService.idl#209

* add support in the query builder: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/src/nsNavHistory.cpp#3086

* create individual result nodes: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/src/nsNavHistory.cpp#6343

* write tests (example: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/unit/test_history.js#103)

probably a few other places, but that should be enough to get started.

Comment 20

9 years ago
Created attachment 345468 [details] [diff] [review]
Patch for unit tests to use RESULTS_AS_FULL_VISIT

How does this look for the unit tests?

I've switched the first tests to check for RESULTS_AS_FULL_VISIT, and added a new test to check for referrer visit ID.

The comment "using full visit crashes in xpcshell" seems to no longer be true, so the first tests pass ok. Obviously the referrer check will fail until the interface is implemented properly.

It might also be worth adding a unit test to check if the referring visit ID contains the correct URI but I'm not quite sure how to look up a specific visit ID in the Places JS API yet.

Comment 21

9 years ago
> It might also be worth adding a unit test to check if the referring visit ID
> contains the correct URI but I'm not quite sure how to look up a specific visit
> ID in the Places JS API yet.

I understand now that comment #7 implies that this is not yet possible.

Comment 22

9 years ago
Created attachment 346233 [details] [diff] [review]
Implement support for RESULTS_AS_FULL_VISIT with tests

My first attempt at fixing this bug. Note that I am not particularly familiar with the Mozilla source tree, so this involves some fairly cargo-cult duplication of existing code.
Attachment #345468 - Attachment is obsolete: true
Attachment #346233 - Flags: review?

Updated

9 years ago
Attachment #346233 - Flags: review? → review?(dietrich)
some brainstorming: i guess if it would not be better waiting to have a referrer column in history table and return the referrer uri instead of an ever-changing visit id, especially now that we have temp tables and visit ids are spread across the two tables. if the scope for exposing the visit id is getting the referrer uri and the from visit uri why not directly returning uris?
Because of comment 23, let's target this for post 3.1.
Assignee: nobody → zanchey
Status: NEW → ASSIGNED
Component: Places → Places
Flags: blocking-firefox3-
Product: Firefox → Toolkit
QA Contact: places → places
Target Milestone: Firefox 3.1 → mozilla1.9.2a1

Updated

9 years ago
Flags: blocking1.9.2?

Updated

9 years ago
Blocks: 382187
yes, comment #23 is spot on. we don't need to wait for a schema change though. could change it to use URI regardless.
No longer blocks: 437245
Flags: blocking1.9.2? → wanted1.9.2+
Target Milestone: mozilla1.9.2a1 → ---
Comment on attachment 346233 [details] [diff] [review]
Implement support for RESULTS_AS_FULL_VISIT with tests

canceling review. definitely most of the way there. should refactor to use URL instead of visit id.
Attachment #346233 - Flags: review?(dietrich)

Updated

8 years ago
Blocks: 260405

Comment 27

8 years ago
(In reply to comment #24)
> Because of comment 23, let's target this for post 3.1.

So 3.6 has branched, any chance of this happening for 3.7?

Comment 28

8 years ago
My understanding at this point is that visit IDs are no longer a reliable value, so making this work would require:
 - updating the schema to use a URI instead of a place ID (although comment #25 says we don't need this? I'm not sure how)
 - updating nsNavHistory::AddVisit and InternalAddVisit to support URIs instead of place IDs
 - rewriting patch to unbitrot and use URIs instead of visit IDs
 - updating the API documentation - RESULTS_AS_FULL_VISIT has never been implemented properly so changes shouldn't break things?

This is beyond my ken, unfortunately.
Assignee: zanchey → nobody
Status: ASSIGNED → NEW
(In reply to comment #28)
>  - updating the API documentation - RESULTS_AS_FULL_VISIT has never been
> implemented properly so changes shouldn't break things?

the plan is to merge FULL_VISIT in VISIT results, providing missing infos and getting rid of a useless result type

Comment 30

8 years ago
Created attachment 421273 [details]
sql to construct visit path

The author of https://addons.mozilla.org/en-US/firefox/addon/14650 has taken David's patch and used it to construct the visit path. I've extracted the relevant part from voyage-0.5-fx/modules/HistoryReader.jsm

Updated

8 years ago
Blocks: 554414
Since this is the reason for ln 154 of services/sync/modules/engines/history.js, I'm curious what happened to this. 

:mak, did FULL_VISIT ever get merged into VISIT results?
(In reply to :Ally Naaktgeboren from comment #31)
> :mak, did FULL_VISIT ever get merged into VISIT results?

not yet, still to do
we are not going to expose visit ids, if there's request to get a "visit path", would be better to ask for an API to get it.
Also, FULL_VISIT nodes have been removed.
Status: NEW → RESOLVED
Last Resolved: 12 years ago5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.