mozilla
/
pjs
зеркало из https://github.com/mozilla/pjs.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 631374 - Adding or removing tags in the selector listbox always scrolls to top. 

r=dietrich
This commit is contained in:
Marco Bonardo 2011-05-05 13:14:21 +02:00
Родитель 5f731547bd
Коммит a13a8a0b31
6 изменённых файлов: 268 добавлений и 9 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

22
browser/components/places/content/editBookmarkOverlay.js
Просмотреть файл

@ -803,6 +803,12 @@ var gEditItemOverlay = {
if (tagsSelectorRow.collapsed)
return;
// Save the current scroll position and restore it after the rebuild.
let firstIndex = tagsSelector.getIndexOfFirstVisibleRow();
let selectedIndex = tagsSelector.selectedIndex;
let selectedTag = selectedIndex >= 0 ? tagsSelector.selectedItem.label
: null;
while (tagsSelector.hasChildNodes())
tagsSelector.removeChild(tagsSelector.lastChild);
@ -815,8 +821,22 @@ var gEditItemOverlay = {
elt.setAttribute("label", tag);
if (tagsInField.indexOf(tag) != -1)
elt.setAttribute("checked", "true");
tagsSelector.appendChild(elt);
if (selectedTag === tag)
selectedIndex = tagsSelector.getIndexOfItem(elt);
}
// Restore position.
// The listbox allows to scroll only if the required offset doesn't
// overflow its capacity, thus need to adjust the index for removals.
firstIndex =
Math.min(firstIndex,
tagsSelector.itemCount - tagsSelector.getNumberOfVisibleRows());
tagsSelector.scrollToIndex(firstIndex);
if (selectedIndex >= 0 && tagsSelector.itemCount > 0) {
selectedIndex = Math.min(selectedIndex, tagsSelector.itemCount - 1);
tagsSelector.selectedIndex = selectedIndex;
tagsSelector.ensureIndexIsVisible(selectedIndex);
}
},

1
browser/components/places/tests/chrome/Makefile.in
Просмотреть файл

@ -52,6 +52,7 @@ _CHROME_TEST_FILES = \
test_bug549192.xul \
test_bug549491.xul \
test_editBookmarkOverlay_tags_liveUpdate.xul \
test_bug631374_tags_selector_scroll.xul \
$(NULL)
libs:: $(_CHROME_TEST_FILES)

156
browser/components/places/tests/chrome/test_bug631374_tags_selector_scroll.xul Normal file
Просмотреть файл

@ -0,0 +1,156 @@
<?xml version="1.0"?>
<!-- Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ -->
<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css"
type="text/css"?>
<?xml-stylesheet href="chrome://browser/skin/places/editBookmarkOverlay.css"?>
<?xml-stylesheet href="chrome://browser/content/places/places.css"?>
<?xml-stylesheet href="chrome://browser/skin/places/places.css"?>
<?xul-overlay href="chrome://browser/content/places/placesOverlay.xul"?>
<?xul-overlay href="chrome://browser/content/places/editBookmarkOverlay.xul"?>
<!DOCTYPE window [
<!ENTITY % editBookmarkOverlayDTD SYSTEM "chrome://browser/locale/places/editBookmarkOverlay.dtd">
%editBookmarkOverlayDTD;
]>
<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
title="Bug 631374 - Editing tags in the selector scrolls up the listbox"
onload="runTest();">
<script type="application/javascript"
src="chrome://mochikit/content/MochiKit/packed.js" />
<script type="application/javascript"
src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
<script type="application/javascript"
src="chrome://browser/content/places/editBookmarkOverlay.js"/>
<body xmlns="http://www.w3.org/1999/xhtml" />
<vbox id="editBookmarkPanelContent"/>
<script type="application/javascript">
<![CDATA[
/**
* This test checks that editing tags doesn't scroll the tags selector
* listbox to wrong positions.
*/
function runTest() {
SimpleTest.waitForExplicitFinish();
Components.utils.import("resource://gre/modules/NetUtil.jsm");
let tags = ["a", "b", "c", "d", "e", "f", "g",
"h", "i", "l", "m", "n", "o", "p"];
// Add a bookmark and tag it.
let uri1 = NetUtil.newURI("http://www1.mozilla.org/");
let id1 = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.toolbarFolderId, uri1,
PlacesUtils.bookmarks.DEFAULT_INDEX, "mozilla"
);
PlacesUtils.tagging.tagURI(uri1, tags);
// Add a second bookmark so that tags won't disappear when unchecked.
let uri2 = NetUtil.newURI("http://www2.mozilla.org/");
let id2 = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.toolbarFolderId, uri2,
PlacesUtils.bookmarks.DEFAULT_INDEX, "mozilla"
);
PlacesUtils.tagging.tagURI(uri2, tags);
// Init panel.
ok(gEditItemOverlay, "gEditItemOverlay is in context");
gEditItemOverlay.initPanel(id1);
ok(gEditItemOverlay._initialized, "gEditItemOverlay is initialized");
// Wait for the tags selector to be open.
let tagsSelectorRow = document.getElementById("editBMPanel_tagsSelectorRow");
tagsSelectorRow.addEventListener("DOMAttrModified", function (event) {
tagsSelectorRow.removeEventListener("DOMAttrModified", arguments.callee, false);
SimpleTest.executeSoon(function () {
let tagsSelector = document.getElementById("editBMPanel_tagsSelector");
// Go by two so there is some untouched tag in the middle.
for (let i = 8; i < tags.length; i += 2) {
tagsSelector.selectedIndex = i;
let listItem = tagsSelector.selectedItem;
SimpleTest.isnot(listItem, null, "Valid listItem found");
tagsSelector.ensureElementIsVisible(listItem);
let visibleIndex = tagsSelector.getIndexOfFirstVisibleRow();
SimpleTest.ok(listItem.checked, "Item is checked " + i);
let selectedTag = listItem.label;
// Uncheck the tag.
listItem.checked = false;
SimpleTest.is(visibleIndex,
tagsSelector.getIndexOfFirstVisibleRow(),
"Scroll position did not change");
// The listbox is rebuilt, so we have to get the new element.
let newItem = tagsSelector.selectedItem;
SimpleTest.isnot(newItem, null, "Valid new listItem found");
SimpleTest.ok(!newItem.checked, "New listItem is unchecked " + i);
SimpleTest.is(newItem.label, selectedTag,
"Correct tag is still selected");
// Check the tag.
newItem.checked = true;
SimpleTest.is(visibleIndex,
tagsSelector.getIndexOfFirstVisibleRow(),
"Scroll position did not change");
}
// Remove the second bookmark, then nuke some of the tags.
PlacesUtils.bookmarks.removeItem(id2);
// Doing this backwords tests more interesting paths.
for (let i = tags.length - 1; i >= 0 ; i -= 2) {
tagsSelector.selectedIndex = i;
let listItem = tagsSelector.selectedItem;
SimpleTest.isnot(listItem, null, "Valid listItem found");
tagsSelector.ensureElementIsVisible(listItem);
let visibleIndex = tagsSelector.getIndexOfFirstVisibleRow();
SimpleTest.ok(listItem.checked, "Item is checked " + i);
let selectedTag = listItem.label;
// Uncheck the tag.
listItem.checked = false;
SimpleTest.is(tagsSelector.getIndexOfFirstVisibleRow(),
Math.max(visibleIndex -1, 0),
"Scroll position is correct");
// The listbox is rebuilt, so we have to get the new element.
let newItem = tagsSelector.selectedItem;
SimpleTest.isnot(newItem, null, "Valid new listItem found");
SimpleTest.ok(newItem.checked, "New listItem is checked " + i);
SimpleTest.is(tagsSelector.selectedItem.label,
tags[Math.min(i + 1, tags.length - 2)],
"The next tag is now selected");
}
// Cleanup.
PlacesUtils.bookmarks.removeItem(id1);
SimpleTest.finish();
});
}, false);
// Open the tags selector.
document.getElementById("editBMPanel_tagsSelectorExpander").doCommand();
}
]]>
</script>
</window>

35
toolkit/components/places/nsNavBookmarks.cpp
Просмотреть файл

@ -146,7 +146,16 @@ public:
{
nsCOMPtr<mozIStorageRow> row;
while (NS_SUCCEEDED(aResultSet->GetNextRow(getter_AddRefs(row))) && row) {
nsresult rv = row->GetInt64(0, &mData.itemId);
// Skip tags, for the use-cases of this async getter they are useless.
PRInt64 grandParentId, tagsFolderId;
nsresult rv = row->GetInt64(1, &grandParentId);
NS_ENSURE_SUCCESS(rv, rv);
rv = mBookmarksSvc->GetTagsFolder(&tagsFolderId);
NS_ENSURE_SUCCESS(rv, rv);
if (grandParentId == tagsFolderId) {
continue;
}
rv = row->GetInt64(0, &mData.itemId);
NS_ENSURE_SUCCESS(rv, rv);
if (mCallback) {
((*mBookmarksSvc).*mCallback)(mData);
@ -302,8 +311,9 @@ nsNavBookmarks::GetStatement(const nsCOMPtr<mozIStorageStatement>& aStmt)
// importing, syncing or due to extensions.
// Note: not using a JOIN is cheaper in this case.
RETURN_IF_STMT(mDBFindURIBookmarks, NS_LITERAL_CSTRING(
"SELECT b.id "
"SELECT b.id, t.parent "
"FROM moz_bookmarks b "
"JOIN moz_bookmarks t on t.id = b.parent "
"WHERE b.fk = (SELECT id FROM moz_places WHERE url = :page_url) "
"ORDER BY b.lastModified DESC, b.id DESC "));
@ -940,7 +950,7 @@ nsNavBookmarks::InsertBookmark(PRInt64 aFolder,
if (grandParentId == mTagsRoot) {
// query for all bookmarks for that URI, notify for each
nsTArray<PRInt64> bookmarks;
rv = GetBookmarkIdsForURITArray(aURI, bookmarks);
rv = GetBookmarkIdsForURITArray(aURI, bookmarks, true);
NS_ENSURE_SUCCESS(rv, rv);
if (bookmarks.Length()) {
@ -1073,7 +1083,7 @@ nsNavBookmarks::RemoveItem(PRInt64 aItemId)
rv = NS_NewURI(getter_AddRefs(uri), spec);
NS_ENSURE_SUCCESS(rv, rv);
nsTArray<PRInt64> bookmarks;
rv = GetBookmarkIdsForURITArray(uri, bookmarks);
rv = GetBookmarkIdsForURITArray(uri, bookmarks, true);
NS_ENSURE_SUCCESS(rv, rv);
for (PRUint32 i = 0; i < bookmarks.Length(); i++) {
@ -1659,7 +1669,7 @@ nsNavBookmarks::RemoveFolderChildren(PRInt64 aFolderId)
NS_ENSURE_SUCCESS(rv, rv);
nsTArray<PRInt64> bookmarks;
rv = GetBookmarkIdsForURITArray(uri, bookmarks);
rv = GetBookmarkIdsForURITArray(uri, bookmarks, true);
NS_ENSURE_SUCCESS(rv, rv);
if (bookmarks.Length()) {
@ -2549,7 +2559,8 @@ nsNavBookmarks::GetFolderIdForItem(PRInt64 aItemId, PRInt64* aFolderId)
nsresult
nsNavBookmarks::GetBookmarkIdsForURITArray(nsIURI* aURI,
nsTArray<PRInt64>& aResult)
nsTArray<PRInt64>& aResult,
bool aSkipTags)
{
NS_ENSURE_ARG(aURI);
@ -2559,6 +2570,15 @@ nsNavBookmarks::GetBookmarkIdsForURITArray(nsIURI* aURI,
PRBool more;
while (NS_SUCCEEDED((rv = stmt->ExecuteStep(&more))) && more) {
if (aSkipTags) {
// Skip tags, for the use-cases of this async getter they are useless.
PRInt64 grandParentId;
nsresult rv = stmt->GetInt64(1, &grandParentId);
NS_ENSURE_SUCCESS(rv, rv);
if (grandParentId == mTagsRoot) {
continue;
}
}
PRInt64 bookmarkId;
rv = stmt->GetInt64(kFindBookmarksIndex_ID, &bookmarkId);
NS_ENSURE_SUCCESS(rv, rv);
@ -2583,7 +2603,8 @@ nsNavBookmarks::GetBookmarkIdsForURI(nsIURI* aURI, PRUint32* aCount,
nsTArray<PRInt64> bookmarks;
// Get the information from the DB as a TArray
nsresult rv = GetBookmarkIdsForURITArray(aURI, bookmarks);
// TODO (bug 653816): make this API skip tags by default.
nsresult rv = GetBookmarkIdsForURITArray(aURI, bookmarks, false);
NS_ENSURE_SUCCESS(rv, rv);
// Copy the results into a new array for output

10
toolkit/components/places/nsNavBookmarks.h
Просмотреть файл

@ -386,9 +386,17 @@ private:
* TArray version of getBookmarksIdForURI for ease of use in C++ code.
* Pass in a reference to a TArray; it will get filled with the
* resulting list of bookmark IDs.
*
* @param aURI
* URI to get bookmarks for.
* @param aResult
* Array of bookmark ids.
* @param aSkipTags
* If true ids of tags-as-bookmarks entries will be excluded.
*/
nsresult GetBookmarkIdsForURITArray(nsIURI* aURI,
nsTArray<PRInt64>& aResult);
nsTArray<PRInt64>& aResult,
bool aSkipTags);
PRInt64 RecursiveFindRedirectedBookmark(PRInt64 aPlaceId);

53
toolkit/components/places/tests/unit/test_onItemChanged_tags.js Normal file
Просмотреть файл

@ -0,0 +1,53 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
// This test checks that changing a tag for a bookmark with multiple tags
// notifies OnItemChanged("tags") only once, and not once per tag.
function run_test() {
do_test_pending();
let tags = ["a", "b", "c"];
let uri = NetUtil.newURI("http://1.moz.org/");
let id = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.unfiledBookmarksFolderId, uri,
PlacesUtils.bookmarks.DEFAULT_INDEX, "Bookmark 1"
);
PlacesUtils.tagging.tagURI(uri, tags);
let bookmarksObserver = {
QueryInterface: XPCOMUtils.generateQI([
Ci.nsINavBookmarkObserver
]),
_changedCount: 0,
onItemChanged: function (aItemId, aProperty, aIsAnnotationProperty, aValue,
aLastModified, aItemType) {
if (aProperty == "tags") {
do_check_eq(aItemId, id);
this._changedCount++;
}
},
onItemRemoved: function (aItemId, aParentId, aIndex, aItemType) {
if (aItemId == id) {
PlacesUtils.bookmarks.removeObserver(this);
do_check_eq(this._changedCount, 2);
do_test_finished();
}
},
onItemAdded: function () {},
onBeginUpdateBatch: function () {},
onEndUpdateBatch: function () {},
onBeforeItemRemoved: function () {},
onItemVisited: function () {},
onItemMoved: function () {},
};
PlacesUtils.bookmarks.addObserver(bookmarksObserver, false);
PlacesUtils.tagging.tagURI(uri, ["d"]);
PlacesUtils.tagging.tagURI(uri, ["e"]);
PlacesUtils.bookmarks.removeItem(id);
}