The default bug view has changed. See this FAQ.

Defect - App bar pin tile icon refreshes after it is displayed instead of before

VERIFIED FIXED in Firefox 25

Status

Firefox for Metro
Browser
P3
normal
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: bbondy, Assigned: kjozwiak)

Tracking

unspecified
Firefox 25
All
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
1. Select a tile and pin it.
2. Select a different tile that is unpinned, notice that pin icon shows up.
3. Unselect that tile
4. Select the tile that was previously pinned

Actual results: You'll see the appbar come up with the pin icon, and then after it is up it changes to the unpin icon
Expected result: It should just show the unpin icon right away and not change after it has come up.

Updated

4 years ago
Blocks: 859003
Whiteboard: feature=defect c=Awesome_screen u=metro_firefox_user p= → feature=defect c=Awesome_screen u=metro_firefox_user p=0
(Reporter)

Comment 1

4 years ago
p=1
Priority: P1 → P3
(Reporter)

Updated

4 years ago
Assignee: nobody → kamiljoz
(Assignee)

Comment 2

4 years ago
Created attachment 776898 [details] [diff] [review]
fixes the pin issue

other than some automated tests, this is my first patch that seems to fix the problem described by Brian in comment 0
Attachment #776898 - Flags: review?(mbrubeck)
Comment on attachment 776898 [details] [diff] [review]
fixes the pin issue

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

Looks great.  Thanks!

::: browser/metro/base/content/appbar.js
@@ +159,5 @@
>    },
>  
>    showContextualActions: function(aVerbs) {
> +    // When the appbar is not visible, we want the icons to refresh right away
> +    var immediate = !Elements.contextappbar.isShowing;

nit: Please use "let" instead of "var".
Attachment #776898 - Flags: review?(mbrubeck) → review+
(Assignee)

Comment 4

4 years ago
Created attachment 777522 [details] [diff] [review]
fixes the pin issue v2

fixed the small nit mentioned in comment 3
Attachment #777522 - Flags: review+
(Reporter)

Comment 5

4 years ago
\o/ :D Nice job!

I pushed to try here:
https://tbpl.mozilla.org/?tree=Try&rev=c070a75e1da0

This will run through the metro tests to make sure nothing got broken in the tests or the app. 

I'll check if that passes tomorrow and if so I'll push it over to mozilla-central.
(Assignee)

Comment 6

4 years ago
Thanks Brian! Appreciate it
(Reporter)

Comment 7

4 years ago
All green, nice job!
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f7bf77ccafb
Whiteboard: feature=defect c=Awesome_screen u=metro_firefox_user p=0 → feature=defect c=Awesome_screen u=metro_firefox_user p=1
Target Milestone: --- → Firefox 25

Updated

4 years ago
Blocks: 886790
No longer blocks: 859003
Status: NEW → ASSIGNED
QA Contact: jbecerra
https://hg.mozilla.org/mozilla-central/rev/0f7bf77ccafb
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Mozilla/5.0 (Windows NT 6.2; rv:25.0) Gecko/20130728 Firefox/25.0

Verified as fixed on the latest Nightly build using the steps from the description.
Status: RESOLVED → VERIFIED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.