927239 - Defect - Context menu items keep highlight after being selected

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Input, defect, P2)

Description

[Kamil Jozwiak [:kjozwiak]]

Attachment: item in the context menu still highlighted

If a user taps & holds either an image or a link and receives the context menu, the selection will stay highlighted if the user taps, holds and then moves away from the context menu. If you dismiss and then get another context menu, you'll notice that the selection is still highlighted from the previous selection.

- Attached a screenshot to illustrate the issue (item was selected with a new context menu)

Steps to reproduce the issue:

1) Open Firefox Metro
2) Go to wikipedia.org
3) Get a context menu by either tapping on a link or image
4) Tap & hold a context menu item and it will be highlighted black, while still having your finger on the screen, move away from the context menu and let go (notice that the item is still highlighted)
5) Dismiss the context menu and then tap and hold another link. You should see that the one of the items in the context menu will still be highlighted from the last time

Current Behavior:

- items under the context menu stay highlighted if you tap, hold and then slide your finger away from the context menu. The user will still have an item highlighted when they dismiss & get new context menu

Expected Behavior:

- items should be cleared when the context menu is dismissed. When a new context menu appears, it should be cleared of any previous selections

Comment 1

[Sam Foster [:sfoster] (he/him)]

I'm not able to reproduce this. First, using touch (not mouse) I see no highlight states in the context menu, when I tap+hold on a link or image. Perhaps this is a bug. 
Using the mouse, the black highlight seems to revert correctly when the menu is dismissed. I'm using a Lenovo X1 Carbon which I think is the same machine as :kjozwiak is using?

Comment 2

[Kamil Jozwiak [:kjozwiak]]

Attachment: video of issue occurring

I went through it and reproduced the issue with today's build, I added a small video on how to reproduce the problem. When the context menu appears, I tap, hold and then swipe away from the context menu and then let go (the item will stay highlighted).

Hope the video helps! let me know if you still cant reproduce it.

Comment 3

[Sam Foster [:sfoster] (he/him)]

Ok, now I see the highlight and the behavior you describe. Thanks, I'll look into it.

Comment 4

[Marina Samuel [:emtwo]]

If it helps at all, I noticed that I'm only able to reproduce this bug with apzc on.

Comment 5

[Jim Mathies [:jimm]]

This appears to be fixed on latest tip. Kamil, mind confirming?

Note we still have a lost tap / touch problem to deal with. Sometimes taps just don't get picked up on the menu and the context stays open. I think this is related to apz. It's also filed under a different bug.

Comment 6

[Marco Mucci [:MarcoM]]

Hey Jim, can you provide a point estimate.

Comment 7

[Kamil Jozwiak [:kjozwiak]]

Jim,

I went through the issue again with the latest nightly and could still reproduce the problem on several websites, used the following build: (used the same steps from comment #0)
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-10-28-03-02-05-mozilla-central/

Please let me know if there's another build that I should have tested with!

Comment 8

[Jim Mathies [:jimm]]

fixed in 10-31 nightly.

Comment 9

[Sam Foster [:sfoster] (he/him)]

I'm still seeing this with these steps: 

* Load any page
* Right-click or long-tap a link on the page to bring up the link context menu
* Select open in new tab (or any of the options)
* Right-click again on the link (or any link) to re-open the context menu

Expected results: 
* None of the context menu options should be pre-selected when the menu shows

Actual results: 
* The option previously selected when the context menu was last opened is selected (highlighted orange)

Comment 10

[Sam Foster [:sfoster] (he/him)]

Attachment: remove selected attribute from menu items when hiding context menu

Not sure if the earlier issue was just masking this one, or if this actually belongs in its own bug. However, afaict hide() is the right place to reset the state of those commands/richlistitems.

Comment 11

[Sam Foster [:sfoster] (he/him)]

Attachment: remove selected attribute from menu items when hiding context menu

sigh. Patch clean-up, missed a qrefresh.

Comment 12

[Jim Mathies [:jimm]]

Comment on attachment 829544 [details] [diff] [review]
remove selected attribute from menu items when hiding context menu

wfm!

Comment 13

[Sam Foster [:sfoster] (he/him)]

selected attribute patch landed on fx-team https://hg.mozilla.org/integration/fx-team/rev/ea7a81a92657

there's still an event-related issue though, if you tap a menu item then drag away before dismissing the menu the :hover state never gets cleared, so the item is highlighted grey when the menu is next opened.

Comment 14

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/ea7a81a92657

Comment 15

[Kamil Jozwiak [:kjozwiak]]

As Sam mentioned in comment #13, there's still an event issue if you follow his steps or the steps in comment #0. I can still reproduce the issue using the latest nightly build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-11-10-03-02-05-mozilla-central/

Comment 16

[Jim Mathies [:jimm]]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #15)
> As Sam mentioned in comment #13, there's still an event issue if you follow
> his steps or the steps in comment #0. I can still reproduce the issue using
> the latest nightly build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-11-10-03-02-05-
> mozilla-central/

This seems like a corner case - you have to slide your finger off the context menu to make this happen.

Comment 17

[Sam Foster [:sfoster] (he/him)]

Follow-up bug filed for the :hover issue at bug 937229.

Comment 18

[Manuela Muntean [Away]]

Verified as fixed for iteration #19, with latest Nightly from 2013-12-01 on Win 8 64-bit, by following the STR from comment 9.

This also needs device verification.