Closed Bug 967391 Opened 6 years ago Closed 6 years ago

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


(Firefox :: General, defect, P2)




Firefox 30
Tracking Status
firefox29 --- verified


(Reporter: MattN, Assigned: MattN)




(1 file, 1 obsolete file)

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


1) Visit the Australis update page
2) Start the tour and enter the step where the menu panel shows, highlighting 
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
Whiteboard: [can-fix-on-aurora]
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)
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+
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.
Closed: 6 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.
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).
Keywords: verifyme
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.