Closed Bug 867163 Opened 7 years ago Closed 7 years ago

Defect - Restore tile button doesn't work when context app bar is visible on a second time

Categories

(Firefox for Metro Graveyard :: Browser, defect, P1)

All
Windows 8.1
defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 23

People

(Reporter: virgil.dicu, Assigned: mbrubeck)

References

Details

(Whiteboard: feature=defect c=Awesome_screen u=metro_firefox_user p=1)

Attachments

(1 file, 1 obsolete file)

Mozilla/5.0 (Windows NT 6.2; WOW64; rv:23.0) Gecko/20130429 Firefox/23.0

1. Select a tile/group of tiles from Top Sites
2. Remove that tile by clicking the "Hide" button from context app bar
3. Right click on Awesome screen to make context app bar disappear
4. Right click again to re-make context app bar visible
5. Click on "Restore" button

Expected result:
Tile/group of tile should be restored.

Found while verifying bug 831918.
Whiteboard: feature=defect c=Awesome_screen u=metro_firefox_user p=0
I think the bug here is that once the context app bar is dismissed the first time, the restore button should go away. The intention is that you just get the one shot at the "undo hide" action.
I had the 'restore not going away' issue when working on bug 831916 too. I fixed it by sending a "MozContextActionsChange" event when the appbar dismisses. I'm not sure if that's the best way or if the appbar should be smarter about it. See https://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/bookmarks.js?force=1#361
I think the appbar should automatically clear itself.
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
Whiteboard: feature=defect c=Awesome_screen u=metro_firefox_user p=0 → feature=defect c=Awesome_screen u=metro_firefox_user p=1
Matt's Point Estimate = 1.
Whiteboard: feature=defect c=Awesome_screen u=metro_firefox_user p=1 → feature=defect c=Awesome_screen u=metro_firefox_user p=0
Point estimate = 1
Blocks: metrov1it7
No longer blocks: metrov1defect&change
Priority: -- → P1
QA Contact: jbecerra
Whiteboard: feature=defect c=Awesome_screen u=metro_firefox_user p=0 → feature=defect c=Awesome_screen u=metro_firefox_user p=1
Attached patch WIP (obsolete) — Splinter Review
Attached patch patchSplinter Review
Any time the appbar dismisses, automatically clear the selection and the context actions.
Attachment #745879 - Attachment is obsolete: true
Attachment #745973 - Flags: review?(sfoster)
Comment on attachment 745973 [details] [diff] [review]
patch

Review of attachment 745973 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good and works well. Thanks for picking this up.

::: browser/metro/base/content/appbar.js
@@ +210,5 @@
>      });
>    },
> +
> +  clearContextualActions: function() {
> +    this.showContextualActions([]);

Its a bit weird that we have to send a magical empty array to clear contextual actions (I know, I did this) but it works and we can refine later if the need arises
Attachment #745973 - Flags: review?(sfoster) → review+
https://hg.mozilla.org/mozilla-central/rev/c39614efd263
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
For testing and verification.
Flags: needinfo?(kamiljoz)
Flags: needinfo?(kamiljoz) → needinfo?(mozbugs.retornam)
Flags: needinfo?(mozbugs.retornam) → needinfo?(kamiljoz)
Went through the following "Defect" and everything passed without any issues during Testing/Verification. Used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-05-16-03-10-04-mozilla-central/

- Selected several tiles from "Top Sites" and removed them. Selected "Restore" and ensured that all the tiles that have been removed have been restored correctly
- Selected all of the tiles from "Top Sites" and removed them. Selected "Restore" and ensured that all the tiles that have been removed have been restored correctly
- Removed several tiles from "Top Sites" and then dismissed the "App Bar", ensured that you cannot restore the tiles as per Comment 1
- Ensured that if you have a large list of tiles under "Top Sites", the removed ones are replaced with others without any issues
- Ensured that if you remove all of the tiles from "Top Sites" and they are replaced with other tiles, selecting "restore" will restore the other ones without any issues
- Ensured that when you remove several tiles from "Top Sites" and then close Firefox Metro, the removed tiles are not reloaded when re-opening Firefox Metro
Status: RESOLVED → VERIFIED
Flags: needinfo?(kamiljoz)
Blocks: 906910
User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130819030205
Built from http://hg.mozilla.org/mozilla-central/rev/c8c9bd74cc40

WFM
Tested on windows 8 using latest nightly for iteration-12. Followed steps provided in comment12 and got expected result except for one. I have filed bug 906910.
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.