Bug 581253 - Cannot remove a bookmark with a tag via 'Edit this bookmark' menu. r=mak a=blocking

This commit is contained in:
Asaf Romano 2010-10-14 10:46:32 +02:00
parent a16956dc0f
commit a954d32efa
5 changed files with 106 additions and 132 deletions

View File

@ -96,13 +96,33 @@ var StarUI = {
if (aEvent.originalTarget == this.panel) {
if (!this._element("editBookmarkPanelContent").hidden)
this.quitEditMode();
this._restoreCommandsState();
this._itemId = -1;
this._uri = null;
if (this._batching) {
PlacesUIUtils.ptm.endBatch();
this._batching = false;
}
switch (this._actionOnHide) {
case "cancel": {
PlacesUIUtils.ptm.undoTransaction();
break;
}
case "remove": {
// Remove all bookmarks for the bookmark's url, this also removes
// the tags for the url.
PlacesUIUtils.ptm.beginBatch();
let itemIds = PlacesUtils.getBookmarksForURI(this._uriForRemoval);
for (let i = 0; i < itemIds.length; i++) {
let txn = PlacesUIUtils.ptm.removeItem(itemIds[i]);
PlacesUIUtils.ptm.doTransaction(txn);
}
PlacesUIUtils.ptm.endBatch();
break;
}
}
this._actionOnHide = "";
}
break;
case "keypress":
@ -185,12 +205,9 @@ var StarUI = {
gNavigatorBundle.getString("editBookmarkPanel.editBookmarkTitle");
// No description; show the Done, Cancel;
// hide the Edit, Undo buttons
this._element("editBookmarkPanelDescription").textContent = "";
this._element("editBookmarkPanelBottomButtons").hidden = false;
this._element("editBookmarkPanelContent").hidden = false;
this._element("editBookmarkPanelEditButton").hidden = true;
this._element("editBookmarkPanelUndoRemoveButton").hidden = true;
// The remove button is shown only if we're not already batching, i.e.
// if the cancel button/ESC does not remove the bookmark.
@ -237,42 +254,6 @@ var StarUI = {
}
},
showPageBookmarkedNotification:
function PCH_showPageBookmarkedNotification(aItemId, aAnchorElement, aPosition) {
this._blockCommands(); // un-done in the popuphiding handler
var brandBundle = this._element("bundle_brand");
var brandShortName = brandBundle.getString("brandShortName");
// "Page Bookmarked" title
this._element("editBookmarkPanelTitle").value =
gNavigatorBundle.getString("editBookmarkPanel.pageBookmarkedTitle");
// description
this._element("editBookmarkPanelDescription").textContent =
gNavigatorBundle.getFormattedString("editBookmarkPanel.pageBookmarkedDescription",
[brandShortName]);
// show the "Edit.." button and the Remove Bookmark button, hide the
// undo-remove-bookmark button.
this._element("editBookmarkPanelEditButton").hidden = false;
this._element("editBookmarkPanelRemoveButton").hidden = false;
this._element("editBookmarkPanelUndoRemoveButton").hidden = true;
// unset the unstarred state, if set
this._element("editBookmarkPanelStarIcon").removeAttribute("unstarred");
this._itemId = aItemId !== undefined ? aItemId : this._itemId;
if (this.panel.state == "closed") {
// Consume dismiss clicks, see bug 400924
this.panel.popupBoxObject
.setConsumeRollupEvent(Ci.nsIPopupBoxObject.ROLLUP_CONSUME);
this.panel.openPopup(aAnchorElement, aPosition, -1, -1);
}
else
this.panel.focus();
},
quitEditMode: function SU_quitEditMode() {
this._element("editBookmarkPanelContent").hidden = true;
this._element("editBookmarkPanelBottomButtons").hidden = true;
@ -284,67 +265,14 @@ var StarUI = {
},
cancelButtonOnCommand: function SU_cancelButtonOnCommand() {
// The order here is important! We have to hide the panel first, otherwise
// changes done as part of Undo may change the panel contents and by
// that force it to commit more transactions
this._actionOnHide = "cancel";
this.panel.hidePopup();
this.endBatch();
PlacesUIUtils.ptm.undoTransaction();
},
removeBookmarkButtonCommand: function SU_removeBookmarkButtonCommand() {
#ifdef ADVANCED_STARRING_UI
// In minimal mode ("page bookmarked" notification), the bookmark
// is removed and the panel is hidden immediately. In full edit mode,
// a "Bookmark Removed" notification along with an Undo button is
// shown
if (this._batching) {
PlacesUIUtils.ptm.endBatch();
PlacesUIUtils.ptm.beginBatch(); // allow undo from within the notification
// "Bookmark Removed" title (the description field is already empty in
// this mode)
this._element("editBookmarkPanelTitle").value =
gNavigatorBundle.getString("editBookmarkPanel.bookmarkedRemovedTitle");
// hide the edit panel
this.quitEditMode();
// Hide the remove bookmark button, show the undo-remove-bookmark
// button.
this._element("editBookmarkPanelUndoRemoveButton").hidden = false;
this._element("editBookmarkPanelRemoveButton").hidden = true;
this._element("editBookmarkPanelStarIcon").setAttribute("unstarred", "true");
this.panel.focus();
}
#endif
// cache its uri so we can get the new itemId in the case of undo
this._uri = PlacesUtils.bookmarks.getBookmarkURI(this._itemId);
// remove all bookmarks for the bookmark's url, this also removes
// the tags for the url
var itemIds = PlacesUtils.getBookmarksForURI(this._uri);
for (var i=0; i < itemIds.length; i++) {
var txn = PlacesUIUtils.ptm.removeItem(itemIds[i]);
PlacesUIUtils.ptm.doTransaction(txn);
}
#ifdef ADVANCED_STARRING_UI
// hidePopup resets our itemId, thus we call it only after removing
// the bookmark
if (!this._batching)
#endif
this.panel.hidePopup();
},
undoRemoveBookmarkCommand: function SU_undoRemoveBookmarkCommand() {
// restore the bookmark by undoing the last transaction and go back
// to the edit state
this.endBatch();
PlacesUIUtils.ptm.undoTransaction();
this._itemId = PlacesUtils.getMostRecentBookmarkForURI(this._uri);
this.showEditBookmarkPopup();
this._uriForRemoval = PlacesUtils.bookmarks.getBookmarkURI(this._itemId);
this._actionOnHide = "remove";
this.panel.hidePopup();
},
beginBatch: function SU_beginBatch() {
@ -352,13 +280,6 @@ var StarUI = {
PlacesUIUtils.ptm.beginBatch();
this._batching = true;
}
},
endBatch: function SU_endBatch() {
if (this._batching) {
PlacesUIUtils.ptm.endBatch();
this._batching = false;
}
}
}
@ -429,10 +350,6 @@ var PlacesCommandHook = {
var position = (getComputedStyle(gNavToolbox, "").direction == "rtl") ? 'after_start' : 'after_end';
if (aShowEditUI)
StarUI.showEditBookmarkPopup(itemId, starIcon, position);
#ifdef ADVANCED_STARRING_UI
else
StarUI.showPageBookmarkedNotification(itemId, starIcon, position);
#endif
return;
}
}
@ -550,15 +467,6 @@ var PlacesCommandHook = {
organizer.PlacesOrganizer.selectLeftPaneQuery(aLeftPaneRoot);
organizer.focus();
}
},
deleteButtonOnCommand: function PCH_deleteButtonCommand() {
PlacesUtils.bookmarks.removeItem(gEditItemOverlay.itemId);
// remove all tags for the associated url
PlacesUtils.tagging.untagURI(gEditItemOverlay._uri, null);
this.panel.hidePopup();
}
};

View File

@ -181,21 +181,10 @@
<label id="editBookmarkPanelTitle"/>
<description id="editBookmarkPanelDescription"/>
<hbox>
<button id="editBookmarkPanelUndoRemoveButton"
class="editBookmarkPanelHeaderButton"
hidden="true"
oncommand="StarUI.undoRemoveBookmarkCommand();"
label="&editBookmark.undo.label;"
accesskey="&editBookmark.undo.accessKey;"/>
<button id="editBookmarkPanelRemoveButton"
class="editBookmarkPanelHeaderButton"
oncommand="StarUI.removeBookmarkButtonCommand();"
accesskey="&editBookmark.removeBookmark.accessKey;"/>
<button id="editBookmarkPanelEditButton"
class="editBookmarkPanelHeaderButton"
oncommand="StarUI.editButtonCommand();"
label="&editBookmark.edit.label;"
accesskey="&editBookmark.edit.accessKey;"/>
</hbox>
</vbox>
</row>

View File

@ -149,6 +149,7 @@ _BROWSER_FILES = \
browser_bug579872.js \
browser_bug580956.js \
browser_bug581242.js \
browser_bug581253.js \
browser_bug581947.js \
browser_bug585830.js \
browser_bug592338.js \

View File

@ -0,0 +1,80 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/
*/
let testURL = "data:text/plain,nothing but plain text";
let testTag = "581253_tag";
let starButton = document.getElementById("star-button");
function test() {
waitForExplicitFinish();
let tab = gBrowser.selectedTab = gBrowser.addTab();
tab.linkedBrowser.addEventListener("load", (function(event) {
tab.linkedBrowser.removeEventListener("load", arguments.callee, true);
let uri = makeURI(testURL);
let bmTxn =
new PlacesCreateBookmarkTransaction(uri,
PlacesUtils.unfiledBookmarksFolderId,
-1, "", null, []);
PlacesUtils.transactionManager.doTransaction(bmTxn);
ok(PlacesUtils.bookmarks.isBookmarked(uri), "the test url is bookmarked");
ok(starButton.getAttribute("starred") == "true",
"star button indicates that the page is bookmarked");
let tagTxn = new PlacesTagURITransaction(uri, [testTag]);
PlacesUtils.transactionManager.doTransaction(tagTxn);
StarUI.panel.addEventListener("popupshown", onPanelShown, false);
starButton.click();
}), true);
content.location = testURL;
}
function onPanelShown(aEvent) {
if (aEvent.target == StarUI.panel) {
StarUI.panel.removeEventListener("popupshown", arguments.callee, false);
let tagsField = document.getElementById("editBMPanel_tagsField");
ok(tagsField.value == testTag, "tags field value was set");
tagsField.focus();
StarUI.panel.addEventListener("popuphidden", onPanelHidden, false);
let removeButton = document.getElementById("editBookmarkPanelRemoveButton");
removeButton.click();
}
}
/**
* Clears history invoking callback when done.
*/
function waitForClearHistory(aCallback)
{
let observer = {
observe: function(aSubject, aTopic, aData)
{
Services.obs.removeObserver(this, PlacesUtils.TOPIC_EXPIRATION_FINISHED);
aCallback(aSubject, aTopic, aData);
}
};
Services.obs.addObserver(observer, PlacesUtils.TOPIC_EXPIRATION_FINISHED, false);
PlacesUtils.bhistory.removeAllPages();
}
function onPanelHidden(aEvent) {
if (aEvent.target == StarUI.panel) {
StarUI.panel.removeEventListener("popuphidden", arguments.callee, false);
executeSoon(function() {
ok(!PlacesUtils.bookmarks.isBookmarked(makeURI(testURL)),
"the bookmark for the test url has been removed");
ok(!starButton.hasAttribute("starred"),
"star button indicates that the bookmark has been removed");
gBrowser.removeCurrentTab();
waitForClearHistory(finish);
});
}
}

View File

@ -514,10 +514,6 @@ you can use these alternative items. Otherwise, their values should be empty. -
<!ENTITY editBookmark.done.label "Done">
<!ENTITY editBookmark.cancel.label "Cancel">
<!ENTITY editBookmark.removeBookmark.accessKey "R">
<!ENTITY editBookmark.undo.label "Undo">
<!ENTITY editBookmark.undo.accessKey "U">
<!ENTITY editBookmark.edit.label "Edit…">
<!ENTITY editBookmark.edit.accessKey "E">
<!ENTITY identity.unverifiedsite2 "This web site does not supply identity information.">
<!ENTITY identity.connectedTo "You are connected to">