UITour: When opening a sub-view, annotations in the panels initial view should be hidden

VERIFIED FIXED in Firefox 29

Status

()

Firefox
General
P2
normal
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: MattN, Assigned: MattN)

Tracking

unspecified
Firefox 30
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox29 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

+++ This bug was initially created as a clone of Bug #949659 +++

STR:

1) Visit the Australis update page
2) Start the tour and enter the step where the menu panel shows, highlighting 
   add-ons.
3) Within the menu panel, click on 'History' or 'Help'

Expected results:
- The annotations in the panel should hide

Actual results:
- The annotation remains visible in the wrong location.

(Quoting Zhenshuo Fang (:fang) - Firefox UX Team from bug 949659 comment #2)
> When you click on an item in the menu that has a
> subview (say history or help), the highlight is still at the same location,
> we should hide it.

Is it fine that if a user closes the subview without choosing an item or closing the menu (e.g. clicks help again) that the annotation doesn't reappear?
Ideally if the user clicks back into the main menu from subview, the highlight should reappear at the same location. We should aim for that and see if it's too much engineering efforts.
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
status-firefox29: --- → affected
Whiteboard: [can-fix-on-aurora]
Created attachment 8371892 [details] [diff] [review]
v.1 Reuse onAppMenuHiding (renamed to hidePanelAnnotations) for ViewShowing. Hide only.

Here is a patch to simply hide the annotations when a subview opens. It doesn't implement comment 1 as I don't think it's worth the effort at this time (for Aurora v.1) and I also wonder if the user would find it in the way while they're exploring as they intentionally opened a subview so probably already saw the highlight.

Tests coming up.
Attachment #8371892 - Flags: review?(bmcbride)
Created attachment 8371923 [details] [diff] [review]
v.1 + Tests -  Reuse onAppMenuHiding (renamed to hidePanelAnnotations) for ViewShowing

Now with tests!
Attachment #8371892 - Attachment is obsolete: true
Attachment #8371892 - Flags: review?(bmcbride)
Attachment #8371923 - Flags: review?(bmcbride)
Comment on attachment 8371923 [details] [diff] [review]
v.1 + Tests -  Reuse onAppMenuHiding (renamed to hidePanelAnnotations) for ViewShowing

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

Well that ended up being much more elegant that I was expecting :)
Attachment #8371923 - Flags: review?(bmcbride) → review+
Comment on attachment 8371923 [details] [diff] [review]
v.1 + Tests -  Reuse onAppMenuHiding (renamed to hidePanelAnnotations) for ViewShowing

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis UITour
User impact if declined: When a panel menu item is highlighted during the tour, clicking an item with a subview (e.g. history, help, bookmarks, developer), would leave the highlight floating out of place in its original position.
Testing completed (on m-c, etc.): Automated mochitest locally
Risk to taking this patch (and alternatives if risky): Very simply change listening for an event new to Australis that is unlikely to conflict with things. Worst case is probably that a highlight would hide prematurely which the user may not even think is a bug.
String or IDL/UUID changes made by this patch: None

(In reply to Blair McBride [:Unfocused] from comment #4)
> Well that ended up being much more elegant that I was expecting :)

Same for me :)
Attachment #8371923 - Flags: approval-mozilla-aurora?
Attachment #8371923 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/integration/fx-team/rev/c326f427d377
Flags: in-testsuite+
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/releases/mozilla-aurora/rev/ae9d9ec82186
status-firefox29: affected → fixed
Follow-up test fixes:
https://hg.mozilla.org/integration/fx-team/rev/243fe57d77dd
https://hg.mozilla.org/releases/mozilla-aurora/rev/91a57ce52263
Luckily this was test-only bustage since our Aurora nightlies ran on a busted cset. We have tree rules that explicitly state that patches are to be on mozilla-central before being uplifted to the release branches. If you're going to ignore that rule and push anyway, you should at least wait until tests are passing or have a green Try run (which clearly wasn't the case here).

This isn't a theoretical issue - we could have very easily shipped busted code to our users because of this. Please respect the rules we have in place.
https://hg.mozilla.org/mozilla-central/rev/c326f427d377
https://hg.mozilla.org/mozilla-central/rev/243fe57d77dd
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 30
I was watching the tree so I wasn't worried about shipping busted code.
You don't control when the nightlies start. And the point still stands, the tree rules are explicit about it.

Updated

4 years ago
Keywords: verifyme
User Agents:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (X11; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0

Verified as fixed on latest Nightly (build ID: 20140302030203) latest Aurora (build ID: 20140303004002).
status-firefox29: fixed → verified
Keywords: verifyme

Updated

4 years ago
Keywords: verifyme

Updated

4 years ago
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.