1817257 - FetchPageDataService should take nsIURI and use loadURI instead of fixupAndLoadURIString

Status: NEW

(Firefox :: Bookmarks & History, defect, P3)

Description

[:Gijs (he/him)]

https://searchfox.org/mozilla-central/rev/b579290e6b830d1b3f0a941203b0c0e1e56c42a3/browser/components/pagedata/PageDataService.sys.mjs#562

AFAICT there are no non-test consumers, also not for queueing the URLs. I don't know if this should be removed instead or if we still have plans for it, or if I'm missing something. If we do have plans for it or I'm missing something, updating it and the tests that rely on it should be reasonably straightforward right now?

Dave, do you know what the status of this code is?

Comment 1

[Dave Townsend [:mossop]]

I'm not aware of plans to use this right now, maybe Stuart does. Otherwise we can probably remove all this code.

Comment 2

[Stuart Colville [:muffinresearch]]

I'm not aware of anything at the moment, what was the original use-case?

Comment 3

[:Gijs (he/him)]

(In reply to scolville from comment #2)

I'm not aware of anything at the moment, what was the original use-case?

I wasn't involved so I don't know. Mossop?

Comment 4

[Dave Townsend [:mossop]]

(In reply to :Gijs (he/him) from comment #3)

(In reply to scolville from comment #2)

I'm not aware of anything at the moment, what was the original use-case?

I wasn't involved so I don't know. Mossop?

The use case was capturing more rich data about pages to display say in the awesomebar/tabs. So better titles that the DOM page title was the first main use but down the road the goal would have been things like product information, prices, etc. for richer shopping experience and identifying similar pages.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:mossop, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

[Dave Townsend [:mossop]]

Marco, is the PageDataService something we're likely to use anytime soon, maybe for search or the address bar? If not we should probably remove it.

Comment 7

[Marco Bonardo [:mak]]

(In reply to Dave Townsend [:mossop] from comment #6)

Marco, is the PageDataService something we're likely to use anytime soon, maybe for search or the address bar? If not we should probably remove it.

That's still TBD, unfortunately.
There are plans to fetch and store distilled page information for Search reasons, but it's unclear whether it will be through PageDataService or through a local small language model, and whether that model will be integrated in PageDataService or be on its own.

I suppose the cost here is packaged code that we're not using, and some minor cost to include the module.
If that's something we want to remove, it would not be a problem to reintroduce the code in the future.

Comment 8

[Dave Townsend [:mossop]]

It's a pretty minor cost and the feature itself is disabled right now. Only cost really is that we run tests on it so using CI time, and I just had to review a patch to it.

Comment 9

[Marco Bonardo [:mak]]

Spending dev time on it is probably not a great idea yet, so that alone may be a reason to remove it.