1556962 - Contextual menu is opening at the wrong position in responsive design mode

Status: RESOLVED DUPLICATE of bug 1625925

(DevTools :: Responsive Design Mode, defect, P3)

Description

[Alin Ilea, Desktop Test Engineering]

Affected versions:

Tested on Nightly 69.0a1 (2019-06-04) and Beta 68.0b7

Affected platforms:

Tested on Windows 7, Windows 10, Mac OS 10.14, Ubuntu 18.04

Prerequisites:
Access FF then trype about:config
Type devtools.responsive.metaViewport.enabled and set the value to "true"

Steps:

1. Open FF and access any website.
2. Open RDM then narrow the RDM window aprox.500px wide (drag it to the right side of the screen).
3. Right click inside the page invoking contextual menu to appear.

Expected result:

The contextual menu successfully appears under the cursor.

Actual result:

The contextual menu is opening shifted on the right side.

Note: The issue occurs just with touch simulation enabled.

Comment 1

[Martin Balfanz [:mbalfanz]]

A testcase that I can reproduce 100% of times on OS X, latest Nightly:

• set devtools.responsive.metaViewport.enabled to true
• open RDM on any bugzilla page (e.g. this one)
• enable touch simulation

The context menu is in the right place when calling it in the top left corner of the viewport, and drifts further away when going to the bottom right corner. It seems like the position is the "original" position, ignoring the zoom that's applied because of the meta viewport.

Comment 2

[Brad Werth [:bradwerth]]

Starting to analyze this. The mouse event location is getting distorted by APZCCallbackHelper::ApplyCallbackTransform.

Comment 3

[Brad Werth [:bradwerth]]

(In reply to Brad Werth [:bradwerth] from comment #2)

Starting to analyze this. The mouse event location is getting distorted by APZCCallbackHelper::ApplyCallbackTransform.

Botond, I'd appreciate your help with this. In my local build, I've confirmed that omitting the call to ApplyCallbackTransform at https://searchfox.org/mozilla-central/source/dom/ipc/BrowserChild.cpp#1666 seems to make this work correctly. I've tested it a small amount with RDM pages that are given different APZ zoom levels and it appears to work correctly in those cases as well. My questions:

1. Can you think of any cases where it would be important for events in RDM documents to use the APZ transform? Basically, what needs to be tested?
2. In order to determine if this is an RDM document, I intend to use Document::IsInRDMPane(). What's the correct way to get a Document pointer associated with this mouse event location? I'm guessing that might need to happen within APZCCallbackHelper::ApplyCallbackTransform based on the nsIContent variable calculated there.
3. Do you agree that this is the right point of intervention to prevent the transform from being applied to events on RDM documents?
4. If we fix this here for mouse events in BrowserChild::HandleRealMouseButtonEvent, we should also fix it for ::DispatchWheelEvent and ::RecvRealTouchEvent. Yes?

Comment 4

[Botond Ballo [:botond]]

My first thought is that we should try to avoid making this logic conditional on RDM. Rather, we should try to understand why the transform is producing incorrect results, and fix it if possible.

Leaving the needinfo on me for now, I'll try to investigate and offer a more specific suggestion.

Comment 5

[Botond Ballo [:botond]]

In particular, I suspect that just not applying the transform will break the ability to left-click on links / buttons inside RDM window.

Comment 6

[Botond Ballo [:botond]]

This affects desktop zooming as well; I filed bug 1557160 for that. I expect the fix there will fix the RDM scenario as well, but let's keep this open for now and double-check when a fix for bug 1557160 lands.

Comment 7

[Botond Ballo [:botond]]

Brad, during your investigation, did you come across the code that observes the event location that's being modified by ApplyCallbackTransform and uses it to position the context menu (presumably a mouse event handler in chrome JS code)? It would be helpful to my investigation of bug 1557160 to know where that code is.

Comment 8

[Brad Werth [:bradwerth]]

(In reply to Botond Ballo [:botond] from comment #7)

Brad, during your investigation, did you come across the code that observes the event location that's being modified by ApplyCallbackTransform and uses it to position the context menu

I've traced it as far as sendContextMenu at https://searchfox.org/mozilla-central/source/devtools/server/actors/emulation/touch-simulator.js#255. From there I'm guessing it goes back to being handled in the Platform code, but I haven't confirmed that.

Comment 9

[Botond Ballo [:botond]]

This is going to be fixed sufficiently for RDM's purposes by bug 1556556. (I'm using bug 1557160 to track an additional issue with context menu positioning which shouldn't affect RDM.)

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 11

[Alin Ilea, Desktop Test Engineering]

Hi,

I cannot reproduce the initial issue anymore by pressing right click to invoking the context menu to appear on latest Nightly 69.0a1 (2019-07-05), but now the same issue occurs when I open the context menu by holding left click in the viewport. The steps are the same but at step 3 instead of right click is hold left click.

Thanks

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Happy to take a patch for 70 but since this is triaged and set to P3 priority I'm setting it as fix-optional.
That will remove the bug from weekly regression triage.

Comment 13

[Rares Doghi, Desktop QA]

Updating the flags for the latest versions, this issue still occurs on our end and in some cases like opening this page in RDM https://es.wikipedia.org/wiki/Wikipedia:Portada#/media/Archivo:Periodic_table_large-es.svg and we use the Kindle Fire HDX - landscape orientation , when the user right clicks on a macbook pro touchpad the context menu appears right under the arrow which automatically selects one of the options.

Comment 16

[Brad Werth [:bradwerth]]

I thought we might have fixed this at some point, but mozregression shows this has remained a problem for at least the last year. Comment 9 says that Bug 1556556 will provide a fix, and there's recent progress on that bug. I hope to be able to test the provisional patch soon.

Comment 17

[Botond Ballo [:botond]]

I retested this with bug 1556556 in the tree, and found that the context menu is still mispositioned. It's likely to be the same issue as bug 1557160, but let's continue to track this separately for now, in case it's not.

Comment 18

[Botond Ballo [:botond]]

I realized that I was testing with apz.allow_zooming=true. The issue does not occur with apz.allow_zooming=false.

I'm somewhat surprised that toggling apz.allow_zooming affects the behaviour of RDM (since the RDM window effectively always has zooming enabled regardless of the value of apz.allow_zooming).

However, this does mean that this bug is resolved for now. It will need to be revisited as part of the effort to enable apz.allow_zooming=true by default.

Comment 19

[Kris Taeleman (:ktaeleman)]

@Martin: DO you think this should block desktop zoom release or can be fixed post release?

Comment 20

[Brad Werth [:bradwerth]]

I'm not convinced this is still occurring. I can't reproduce it. These Steps to Reproduce would certainly be affected by the changes in Bug 1625925. Does it still occur for you?

Comment 21

[Alin Ilea, Desktop Test Engineering]

I can't reproduce the issue anymore in latest Nightly 81.0a1 (2020-08-17). Seems to be fixed now.

Comment 22

[Martin Balfanz [:mbalfanz]]

I also can't reproduce this anymore, so should be good to go. Though, if we had STR, I would consider this blocking.