1479722 - setCharsetForURI should use history.update

Status: RESOLVED FIXED

(Toolkit :: Places, enhancement, P2)

Description

[Marco Bonardo [:mak]]

this is one of the remaining uses of the page annotations API

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1479722 - Rework PlacesUtils.setCharsetForURI to PlacesUIUtils.setCharsetForPage and avoid main thread sync io.

MozReview-Commit-ID: HeO3Mm5Dibo

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 1479722 - Remove nsIAnnotationService::setPageAnnotation and removePageAnnotation.

Depends on D2582

Comment 3

[Mark Banner (:standard8)]

Try push is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce2d671457b86b0fe024b3b003e774888062e3c7

Comment 4

[Phabricator Automation]

Comment on attachment 8996468 [details]
Bug 1479722 - Rework PlacesUtils.setCharsetForURI to PlacesUIUtils.setCharsetForPage and avoid main thread sync io.

Marco Bonardo [::mak] has approved the revision.

https://phabricator.services.mozilla.com/D2582

Comment 5

[Phabricator Automation]

Comment on attachment 8996470 [details]
Bug 1479722 - Remove nsIAnnotationService::setPageAnnotation and removePageAnnotation.

Marco Bonardo [::mak] has approved the revision.

https://phabricator.services.mozilla.com/D2583

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/26c498b422eb
Rework PlacesUtils.setCharsetForURI to PlacesUIUtils.setCharsetForPage and avoid main thread sync io. r=mak

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fde7e80c27a7
Remove nsIAnnotationService::setPageAnnotation and removePageAnnotation. r=mak

Comment 8

[Masatoshi Kimura [:emk]]

Is the charset information still needed in the history? The only remaining user is keyword bookmarks. How about adding charset to moz_keywords instead of annotations?

Comment 9

[Marco Bonardo [:mak]]

(In reply to Masatoshi Kimura [:emk] from comment #8)
> Is the charset information still needed in the history? The only remaining
> user is keyword bookmarks. How about adding charset to moz_keywords instead
> of annotations?

It's indeed an interesting suggestion that I thought about time ago too, once we have an abstraction we can do anything.
There are 2 problems there:
1. we don't know if the user will add a keyword to a bookmark at a later time, thus we have to store the charset regardless
2. the user can still force a charset for a page through the main menubar (though I'm not sure if that info is still used properly)

I think we should see if bug 648398 goes anywhere, anyway it should not be a big deal to keep things as they are for a while.

Comment 10

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/26c498b422eb
https://hg.mozilla.org/mozilla-central/rev/fde7e80c27a7