1254030 - Drag previews are in the wrong place with desktop zooming enabled

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

I was testing out the support from bug 1242690 and saw this with autocomplete popups and with drag previews.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Drag previews are covered by bug 1244284. We can use this bug for autocomplete popups. I imagine select element popups are also affected?

Comment 2

[Botond Ballo [:botond]]

Updating bug title to reflect that we're not actually turning on processing of meta viewport tags as part of desktop zooming.

Comment 3

[Botond Ballo [:botond]]

This is not fixed by bug 1556556 - autocomplete and select popups are still both mispositioned when zoomed. (However, the work in bug 1556556 should make the remaining fix a lot clearer / easier.)

Comment 4

[Botond Ballo [:botond]]

Context menu popups are also affected. This is tracked in bug 1557160, but may have the same root cause as this bug.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Bug 1632268 fixes form autocomplete popup positioning. However on macOS at least drag previews are still wrong. The drag preview appears in the wrong spot initially, but then quickly slides itself over to where the mouse is. So it's not huge problem, but it would be good if it started out in the right place too. I can look into this.

And context menu popups seem ok as far as I can tell, that was probably all fixed by bug 1557160.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

After poking around the code a bit, I think what needs to happen is that the origin of aDragRect needs to get converted from layout to visual space around here, at least for macOS. However this code is running in the parent process, and the layout/visual space conversion information is in the content process. So it's going to be a little tricky...

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Some more digging reveals that it's the content process that's painting the drag image, in the nsDragServiceProxy that lives in the content process. That happens here. That gets shipped to the parent process and received here as a RemoteDragStartData and plumbed into the parent-process drag service here. So that's good news, it makes fixing this bug much more feasible as we can either update the rect or image (or both) or even just record the transform while in the content process and then pass it over to the parent.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

I continued investigating the code related to drag images and positioning. I realized that right now the drag image is generated un-zoomed, which looks a little weird because it doesn't match what's visible on the screen. Chrome does make the drag image zoomed, and it looks nicer.

The drag image is painted using PaintRangePaintInfo which uses some custom displaylist building code here and will effectively omit any nsDisplayAsyncZoom or nsDisplayResolution items. So doing this "deeper" fix and ensuring stuff gets painted at the right zoom level might work better.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Quick update, I have a set of patches that fixes both the size of the drag image and the positioning, at least on my one test case. I suspect it may not work well with dragging things inside scrolled iframes that are zoomed, but I'll test that tomorrow. Stuff so far is at https://github.com/staktrace/gecko-dev/commits/popups

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Apparently I never assigned this bug to myself. Oh well.

Try push with latest cleaned up patches: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=c8f38accc3289d4aa4d7f998a7ca7e54dace7e97
Unless there's unexpected failures the patches should be pretty final. I guess I should probably also test on non-macOS just to be safe...

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1254030 - Scale drag image by APZ zoom. r?botond,tnikkel

When rasterizing the drag image, we pick up the resolution from ancestor
presShells and ensure that the drag image is rasterized at that resolution,
with appropriate limits for memory usage.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1254030 - Fix positioning of drag images with APZ zoom applied. r?botond,tnikkel

This adjusts the position at which the drag images appear when doing drag
actions, so that they appear where you would expect when APZ zoom is applied.
There doesn't seem to be a good way to test this, but I did a bunch of manual
testing, with all the possible expansions of this sentence:
Dragging {a small image,a large image,some text} in {an iframe,the root
content document}, with {,no }zooming applied.
In all cases, the drag image/text should appear such that the part under the
cursor is the same as what was under the cursor on the original rendering of
the page.

Depends on D77435

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1254030 - Fix size of drag image outline when nglayout.enable_drag_images=false. r?botond,tnikkel

The positioning isn't right, but it's not right even without APZ zooming, so
this patch doesn't mess with it.

Depends on D77436

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Updating bug summary to actually match what this bug is for now. Bug 1632268 takes care of other popup things.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/145c05a9fdf0
Scale drag image by APZ zoom. r=botond
https://hg.mozilla.org/integration/autoland/rev/9a2d13756c44
Fix positioning of drag images with APZ zoom applied. r=botond
https://hg.mozilla.org/integration/autoland/rev/432c1dcd0c22
Fix size of drag image outline when nglayout.enable_drag_images=false. r=botond

Comment 16

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/145c05a9fdf0
https://hg.mozilla.org/mozilla-central/rev/9a2d13756c44
https://hg.mozilla.org/mozilla-central/rev/432c1dcd0c22

Comment 17

[Oana Botisan, Desktop QA]

Can you please provide some steps to reproduce because I can't seem to be able to reproduce the issue.

Comment 18

[Kartikaya Gupta (email:[e-mail])]

The problem occurs if you try to drag an image or selected text. When you start dragging, there is a "preview" image that appears near/under the mouse cursor, and with desktop zooming enabled it was initially positioned and sized incorrectly. Here's the specific STR that I used (as best I can remember):

1. Turn on apz.allow_zooming=true
2. Go to https://staktrace.com/kats/contact.php
3. Use desktop zooming to zoom in (I was using pinch gesture on macOS trackpad)
4. Place the mouse cursor over the captcha image (the one with the math question), press mouse button down, and move mouse. Verify that the drag preview image initially appears under the mouse, positioned where the original image is, and then moves around in sync with the mouse. Without these patches, the drag preview would initially appear somewhere else and then slide over to where the mouse cursor was. Also verify the size of the drag image matches the image as visible on the screen (zoomed in). Note that images that are extremely large do still get scaled down to save on memory, that is expected.
5. Repeat step 4 but with some text selection - select some text, and then drag it to get a drag preview image of the selection.

You can try this on a variety of pages with different images and text. Let me know if the STR are not clear or if you still have trouble reproducing the problem.

Comment 19

[Oana Botisan, Desktop QA]

Thank you for the steps, but I still can't reproduce the issue. I will ask another colleague that has access to a laptop with touchscreen and he will verify this bug.

Comment 20

[Ciprian Georgiu, Desktop QA]

I was able to reproduce this bug on a Dell XPS with a touchscreen, and by following the steps from comment 18.

I can confirm that the bug is verified as fixed on latest Beta 79.0b3, under Win 10 x64.