diff --git a/browser/base/content/browser-places.js b/browser/base/content/browser-places.js index c1011fc93fbc..90c9526d3caf 100644 --- a/browser/base/content/browser-places.js +++ b/browser/base/content/browser-places.js @@ -399,9 +399,8 @@ var PlacesCommandHook = { info.guid = await PlacesTransactions.NewBookmark(info).transact(); - // Set the character-set - if (charset && !PrivateBrowsingUtils.isBrowserPrivate(browser)) { - PlacesUtils.setCharsetForURI(makeURI(url.href), charset); + if (charset) { + PlacesUIUtils.setCharsetForPage(url, charset, window).catch(Cu.reportError); } } diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index 7e87f5936089..06b751754389 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -6225,8 +6225,9 @@ function BrowserSetForcedCharacterSet(aCharset) { if (aCharset) { gBrowser.selectedBrowser.characterSet = aCharset; // Save the forced character-set - if (!PrivateBrowsingUtils.isWindowPrivate(window)) - PlacesUtils.setCharsetForURI(getWebNavigation().currentURI, aCharset); + PlacesUIUtils.setCharsetForPage(getWebNavigation().currentURI, + aCharset, + window).catch(Cu.reportError); } BrowserCharsetReload(); } @@ -8244,4 +8245,3 @@ var ConfirmationHint = { return this._message = document.getElementById("confirmation-hint-message"); }, }; - diff --git a/browser/components/places/PlacesUIUtils.jsm b/browser/components/places/PlacesUIUtils.jsm index 974a7f0c8d78..6e89c9097074 100644 --- a/browser/components/places/PlacesUIUtils.jsm +++ b/browser/components/places/PlacesUIUtils.jsm @@ -459,6 +459,32 @@ var PlacesUIUtils = { PlacesUtils.history.markPageAsFollowedLink(this.createFixedURI(aURL)); }, + /** + * Sets the character-set for a page. The character set will not be saved + * if the window is determined to be a private browsing window. + * + * @param {string|URL|nsIURI} url The URL of the page to set the charset on. + * @param {String} charset character-set value. + * @param {window} window The window that the charset is being set from. + * @return {Promise} + */ + async setCharsetForPage(url, charset, window) { + if (PrivateBrowsingUtils.isWindowPrivate(window)) { + return; + } + + // UTF-8 is the default. If we are passed the value then set it to null, + // to ensure any charset is removed from the database. + if (charset.toLowerCase() == "utf-8") { + charset = null; + } + + await PlacesUtils.history.update({ + url, + annotations: new Map([[PlacesUtils.CHARSET_ANNO, charset]]) + }); + }, + /** * Allows opening of javascript/data URI only if the given node is * bookmarked (see bug 224521). diff --git a/browser/components/places/content/bookmarkProperties.js b/browser/components/places/content/bookmarkProperties.js index 46e9638f0486..a6e5862ef7f5 100644 --- a/browser/components/places/content/bookmarkProperties.js +++ b/browser/components/places/content/bookmarkProperties.js @@ -468,8 +468,9 @@ var BookmarkPropertiesPanel = { if (this._postData) info.postData = this._postData; - if (this._charSet && !PrivateBrowsingUtils.isWindowPrivate(window)) - PlacesUtils.setCharsetForURI(this._uri, this._charSet); + if (this._charSet) { + PlacesUIUtils.setCharsetForPage(this._uri, this._charSet, window).catch(Cu.reportError); + } itemGuid = await PlacesTransactions.NewBookmark(info).transact(); } else if (this._itemType == LIVEMARK_CONTAINER) { diff --git a/browser/components/places/tests/unit/test_PUIU_setCharsetForPage.js b/browser/components/places/tests/unit/test_PUIU_setCharsetForPage.js new file mode 100644 index 000000000000..3a0e414f48a7 --- /dev/null +++ b/browser/components/places/tests/unit/test_PUIU_setCharsetForPage.js @@ -0,0 +1,101 @@ +/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */ +/* vim:set ts=2 sw=2 sts=2 et: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const {PrivateBrowsingUtils} = + ChromeUtils.import("resource://gre/modules/PrivateBrowsingUtils.jsm", {}); + +const UTF8 = "UTF-8"; +const UTF16 = "UTF-16"; +const CHARSET_ANNO = PlacesUtils.CHARSET_ANNO; + +const TEST_URI = "http://foo.com"; +const TEST_BOOKMARKED_URI = "http://bar.com"; + +add_task(function setup() { + let savedIsWindowPrivateFunc = PrivateBrowsingUtils.isWindowPrivate; + PrivateBrowsingUtils.isWindowPrivate = () => false; + + registerCleanupFunction(() => { + PrivateBrowsingUtils.isWindowPrivate = savedIsWindowPrivateFunc; + }); +}); + +add_task(async function test_simple_add() { + // add pages to history + await PlacesTestUtils.addVisits(TEST_URI); + await PlacesTestUtils.addVisits(TEST_BOOKMARKED_URI); + + // create bookmarks on TEST_BOOKMARKED_URI + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.unfiledGuid, + url: TEST_BOOKMARKED_URI, + title: TEST_BOOKMARKED_URI.spec, + }); + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + url: TEST_BOOKMARKED_URI, + title: TEST_BOOKMARKED_URI.spec, + }); + + // set charset on not-bookmarked page + await PlacesUIUtils.setCharsetForPage(TEST_URI, UTF16, {}); + // set charset on bookmarked page + await PlacesUIUtils.setCharsetForPage(TEST_BOOKMARKED_URI, UTF16, {}); + + let pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations: true}); + Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), UTF16, + "Should return correct charset for a not-bookmarked page"); + + pageInfo = await PlacesUtils.history.fetch(TEST_BOOKMARKED_URI, {includeAnnotations: true}); + Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), UTF16, + "Should return correct charset for a bookmarked page"); + + await PlacesUtils.history.clear(); + + pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations: true}); + Assert.ok(!pageInfo, "Should not return pageInfo for a page after history cleared"); + + pageInfo = await PlacesUtils.history.fetch(TEST_BOOKMARKED_URI, {includeAnnotations: true}); + Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), UTF16, + "Charset should still be set for a bookmarked page after history clear"); + + await PlacesUIUtils.setCharsetForPage(TEST_BOOKMARKED_URI, ""); + pageInfo = await PlacesUtils.history.fetch(TEST_BOOKMARKED_URI, {includeAnnotations: true}); + Assert.strictEqual(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), undefined, + "Should not have a charset after it has been removed from the page"); +}); + +add_task(async function test_utf8_clears_saved_anno() { + await PlacesUtils.history.clear(); + await PlacesTestUtils.addVisits(TEST_URI); + + // set charset on bookmarked page + await PlacesUIUtils.setCharsetForPage(TEST_URI, UTF16, {}); + + let pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations: true}); + Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), UTF16, + "Should return correct charset for a not-bookmarked page"); + + // Now set the bookmark to a UTF-8 charset. + await PlacesUIUtils.setCharsetForPage(TEST_URI, UTF8, {}); + + pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations: true}); + Assert.strictEqual(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), undefined, + "Should have removed the charset for a UTF-8 page."); +}); + +add_task(async function test_private_browsing_not_saved() { + await PlacesUtils.history.clear(); + await PlacesTestUtils.addVisits(TEST_URI); + + // set charset on bookmarked page, but pretend this is a private browsing window. + PrivateBrowsingUtils.isWindowPrivate = () => true; + await PlacesUIUtils.setCharsetForPage(TEST_URI, UTF16, {}); + + let pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations: true}); + Assert.strictEqual(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), undefined, + "Should not have set the charset in a private browsing window."); +}); diff --git a/browser/components/places/tests/unit/xpcshell.ini b/browser/components/places/tests/unit/xpcshell.ini index 737136e9c38e..d0041c80918d 100644 --- a/browser/components/places/tests/unit/xpcshell.ini +++ b/browser/components/places/tests/unit/xpcshell.ini @@ -18,3 +18,4 @@ support-files = [test_browserGlue_restore.js] [test_clearHistory_shutdown.js] [test_PUIU_batchUpdatesForNode.js] +[test_PUIU_setCharsetForPage.js] diff --git a/toolkit/components/places/Bookmarks.jsm b/toolkit/components/places/Bookmarks.jsm index 7ffba6cdd89e..6b3b26c07a26 100644 --- a/toolkit/components/places/Bookmarks.jsm +++ b/toolkit/components/places/Bookmarks.jsm @@ -1395,7 +1395,7 @@ var Bookmarks = Object.freeze({ JOIN moz_bookmarks c ON c.parent = b.id WHERE p.guid = :tagsGuid GROUP BY name - ORDER BY name COLLATE nocase ASC + ORDER BY name COLLATE nocase ASC `, { tagsGuid: this.tagsGuid }); return rows.map(r => ({ name: r.getResultByName("name"), @@ -1947,7 +1947,17 @@ async function handleBookmarkItemSpecialData(itemId, item) { } if ("charset" in item && item.charset) { try { - await PlacesUtils.setCharsetForURI(NetUtil.newURI(item.url), item.charset); + // UTF-8 is the default. If we are passed the value then set it to null, + // to ensure any charset is removed from the database. + let charset = item.charset; + if (item.charset.toLowerCase() == "utf-8") { + charset = null; + } + + await PlacesUtils.history.update({ + url: item.url, + annotations: new Map([[PlacesUtils.CHARSET_ANNO, charset]]) + }); } catch (ex) { Cu.reportError(`Failed to set charset "${item.charset}" for ${item.url}: ${ex}`); } diff --git a/toolkit/components/places/PlacesUtils.jsm b/toolkit/components/places/PlacesUtils.jsm index fb33ff026eba..ea6728127afc 100644 --- a/toolkit/components/places/PlacesUtils.jsm +++ b/toolkit/components/places/PlacesUtils.jsm @@ -1484,31 +1484,6 @@ var PlacesUtils = { return db.executeBeforeShutdown(name, task); }, - /** - * Sets the character-set for a URI. - * - * @param {nsIURI} aURI - * @param {String} aCharset character-set value. - * @return {Promise} - */ - setCharsetForURI: function PU_setCharsetForURI(aURI, aCharset) { - return new Promise(resolve => { - // Delaying to catch issues with asynchronous behavior while waiting - // to implement asynchronous annotations in bug 699844. - Services.tm.dispatchToMainThread(function() { - if (aCharset && aCharset.length > 0) { - PlacesUtils.annotations.setPageAnnotation( - aURI, PlacesUtils.CHARSET_ANNO, aCharset, 0, - Ci.nsIAnnotationService.EXPIRE_NEVER); - } else { - PlacesUtils.annotations.removePageAnnotation( - aURI, PlacesUtils.CHARSET_ANNO); - } - resolve(); - }); - }); - }, - /** * Gets favicon data for a given page url. * diff --git a/toolkit/components/places/tests/bookmarks/test_1129529.js b/toolkit/components/places/tests/bookmarks/test_1129529.js index 19b209e98b2f..cb68467b6369 100644 --- a/toolkit/components/places/tests/bookmarks/test_1129529.js +++ b/toolkit/components/places/tests/bookmarks/test_1129529.js @@ -24,7 +24,7 @@ add_task(async function() { guid: "___guid1____", index: 0, id: 3, - charset: "UTF-8", + charset: "UTF-16", tags: "tag0", type: "text/x-moz-place", dateAdded: now, @@ -35,7 +35,7 @@ add_task(async function() { guid: "___guid2____", index: 1, id: 4, - charset: "UTF-8", + charset: "UTF-16", tags: "tag1,a" + "0123456789".repeat(10), // 101 chars type: "text/x-moz-place", dateAdded: now, @@ -46,7 +46,7 @@ add_task(async function() { guid: "___guid3____", index: 2, id: 5, - charset: "UTF-8", + charset: "UTF-16", tags: "tag2", type: "text/x-moz-place", dateAdded: now, @@ -67,7 +67,7 @@ add_task(async function() { for (let i = 0; i < unsortedBookmarks.length; ++i) { let bookmark = unsortedBookmarks[i]; - Assert.equal(bookmark.charset, "UTF-8"); + Assert.equal(bookmark.charset, "UTF-16"); Assert.equal(bookmark.dateAdded, now); Assert.equal(bookmark.lastModified, now); Assert.equal(bookmark.uri, "http://test" + i + ".com/"); diff --git a/toolkit/components/places/tests/unit/test_317472.js b/toolkit/components/places/tests/unit/test_317472.js deleted file mode 100644 index 8b6c7f4fe6f9..000000000000 --- a/toolkit/components/places/tests/unit/test_317472.js +++ /dev/null @@ -1,65 +0,0 @@ -/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */ -/* vim:set ts=2 sw=2 sts=2 et: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -const charset = "UTF-8"; -const CHARSET_ANNO = PlacesUtils.CHARSET_ANNO; - -const TEST_URI = uri("http://foo.com"); -const TEST_BOOKMARKED_URI = uri("http://bar.com"); - -add_task(async function test_execute() { - // add pages to history - await PlacesTestUtils.addVisits(TEST_URI); - await PlacesTestUtils.addVisits(TEST_BOOKMARKED_URI); - - // create bookmarks on TEST_BOOKMARKED_URI - await PlacesUtils.bookmarks.insert({ - parentGuid: PlacesUtils.bookmarks.unfiledGuid, - url: TEST_BOOKMARKED_URI, - title: TEST_BOOKMARKED_URI.spec, - }); - await PlacesUtils.bookmarks.insert({ - parentGuid: PlacesUtils.bookmarks.toolbarGuid, - url: TEST_BOOKMARKED_URI, - title: TEST_BOOKMARKED_URI.spec, - }); - - // set charset on not-bookmarked page - await PlacesUtils.setCharsetForURI(TEST_URI, charset); - // set charset on bookmarked page - await PlacesUtils.setCharsetForURI(TEST_BOOKMARKED_URI, charset); - - // check that we have created a page annotation - Assert.equal(PlacesUtils.annotations.getPageAnnotation(TEST_URI, CHARSET_ANNO), charset); - - let pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations: true}); - Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), charset, - "Should return correct charset for a not-bookmarked page"); - - pageInfo = await PlacesUtils.history.fetch(TEST_BOOKMARKED_URI, {includeAnnotations: true}); - Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), charset, - "Should return correct charset for a bookmarked page"); - - await PlacesUtils.history.clear(); - - pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations: true}); - Assert.ok(!pageInfo, "Should not return pageInfo for a page after history cleared"); - - // check that page annotation has been removed - try { - PlacesUtils.annotations.getPageAnnotation(TEST_URI, CHARSET_ANNO); - do_throw("Charset page annotation has not been removed correctly"); - } catch (e) {} - - pageInfo = await PlacesUtils.history.fetch(TEST_BOOKMARKED_URI, {includeAnnotations: true}); - Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), charset, - "Charset should still be set for a bookmarked page after history clear"); - - await PlacesUtils.setCharsetForURI(TEST_BOOKMARKED_URI, ""); - pageInfo = await PlacesUtils.history.fetch(TEST_BOOKMARKED_URI, {includeAnnotations: true}); - Assert.notEqual(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), charset, - "Should not have a charset after it has been removed from the page"); -}); diff --git a/toolkit/components/places/tests/unit/test_bookmarks_html_escape_entities.js b/toolkit/components/places/tests/unit/test_bookmarks_html_escape_entities.js index 0cb67c29f168..4e11e7c367a7 100644 --- a/toolkit/components/places/tests/unit/test_bookmarks_html_escape_entities.js +++ b/toolkit/components/places/tests/unit/test_bookmarks_html_escape_entities.js @@ -25,9 +25,11 @@ add_task(async function() { title: unescaped }); await PlacesUtils.keywords.insert({ url, keyword: unescaped, postData: unescaped }); - let uri = Services.io.newURI(url); - PlacesUtils.tagging.tagURI(uri, [unescaped]); - await PlacesUtils.setCharsetForURI(uri, unescaped); + PlacesUtils.tagging.tagURI(Services.io.newURI(url), [unescaped]); + await PlacesUtils.history.update({ + url, + annotations: new Map([[PlacesUtils.CHARSET_ANNO, unescaped]]), + }); PlacesUtils.annotations.setItemAnnotation( await PlacesUtils.promiseItemId(bm.guid), DESCRIPTION_ANNO, unescaped, 0, PlacesUtils.annotations.EXPIRE_NEVER); diff --git a/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js b/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js index ff0cf5e5962c..3e00065d68bd 100644 --- a/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js +++ b/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js @@ -212,7 +212,10 @@ add_task(async function() { annotations: [{ name: "TestAnnoA", value: "TestVal2"}]}); let urlWithCharsetAndFavicon = uri("http://charset.and.favicon"); await new_bookmark({ parentGuid: folderGuid, url: urlWithCharsetAndFavicon }); - await PlacesUtils.setCharsetForURI(urlWithCharsetAndFavicon, "UTF-8"); + await PlacesUtils.history.update({ + url: urlWithCharsetAndFavicon, + annotations: new Map([[PlacesUtils.CHARSET_ANNO, "UTF-16"]]), + }); await setFaviconForPage(urlWithCharsetAndFavicon, SMALLPNG_DATA_URI); // Test the default places root without specifying it. await test_promiseBookmarksTreeAgainstResult(); diff --git a/toolkit/components/places/tests/unit/xpcshell.ini b/toolkit/components/places/tests/unit/xpcshell.ini index 9f472d224314..c58e0ad0cf16 100644 --- a/toolkit/components/places/tests/unit/xpcshell.ini +++ b/toolkit/components/places/tests/unit/xpcshell.ini @@ -16,7 +16,6 @@ support-files = places.sparse.sqlite [test_000_frecency.js] -[test_317472.js] [test_331487.js] [test_384370.js] [test_385397.js]