933302 - nsDragService::ConstructDragImage doesn't always work properly in @2x mode (Retina Drag Drop)

Status: RESOLVED DUPLICATE of bug 1301673

(Core :: Widget: Cocoa, defect, P2)

Description

[clifford.racz]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.101 Safari/537.36

Steps to reproduce:

1. Select messages with mouse (1 or more) and hold left mouse button
2. Drag over destination/target folder (IMAP, local, same or different account, it doesn't matter)
3. Release left mouse button to initiate move functionality

Note: This is on Mac OS X 10.8.5 and I have not confirmed on other platform(s).  


Actual results:

No visual cue appears to indicate that messages are selected. Messages *do* move.


Expected results:

Users look for the visual cue, and it should appear.  Lack of cue communicates that messages are not selected when they actually are.  Cue should be present.

Comment 1

[Wayne Mery (:wsmwk)]

Do you see this with Thunderbird 24.1 started in safe mode?

(Please refrain from adding unrelated keywords)

Comment 2

[clifford.racz]

My Thunderbird has updated to 24.2.0
This still happens even in safe mode.  I imagine it happened in 24.1 as well but I cannot verify

Comment 3

[Nomis101]

I think this could be a core bug. Yesterday I tried to find out how TB is generating this drag image. For the drag action we call dragservice in msgMail3PaneWindow.js and than the rest (including the image) is done from /widget/cocoa/nsDragService.mm
http://hg.mozilla.org/mozilla-central/file/451f47a70238/widget/cocoa/nsDragService.mm#l130
So I speculate that this ConstructDragImage is not working correctly for HiDPI(?). But if so, we should see similar issues for Firefox.

Comment 4

[Josiah Bruner [:jsbruner]]

I should be able to look at this soon.

Comment 5

[Josiah Bruner [:jsbruner]]

Verified that this happens on TB on OS X 10.9. And, as Nomis suggested, this happens on Firefox as well on a retina display. (I used the bookmarks side bar to test).

Therefore, I'm making this core Widget/Cocoa bug.

Comment 6

[Josiah Bruner [:jsbruner]]

Although I plan to look at the ConstructDragImage code myself, I'm Ccing Markus since I'm sure he knows substantially more about Cocoa-based graphics than I do.

Comment 7

[Josiah Bruner [:jsbruner]]

So I've found a few things so far.

1. The function is not returning anywhere early. It does reach the end of its function and is returning a NSImage of some kind.

2. It correctly gets the scale factor.

3. It does get values from aDragRect->width;| and |aDragRect->height|

3 It's width and height values derived from |aDragRect->width;| and |aDragRect->height| return proper (expected) values.

So most likely it is a matter of using invalid settings for some of the gfx-related things. However, I have no experience with gfx, so this is about as far as I can go.

Comment 8

[Nomis101]

Because you say gfx, I remembered there was the HiDPI Bug 785667 which changes something in gfx for mozilla 18. And indeed, I don't have this issue with Thunderbird 17 (tested 17.0.11esr), but I have this issue with Thunderbird 18 (tested Thunderbird 18.0b1). Maybe I can find a better regression range...

Comment 9

[Nomis101]

(In reply to Nomis101 from comment #8)
> Maybe I can find a better
> regression range...

Hm, this does not help at all.

The last working build is:
20120929030236
http://hg.mozilla.org/mozilla-central/rev/c09a0c022b2e
http://hg.mozilla.org/comm-central/rev/74565e36a305

The first not working build is:
20120930030231
http://hg.mozilla.org/mozilla-central/rev/d912cef9b337
http://hg.mozilla.org/comm-central/rev/57f319d552b5

This is exactly when HiDPI was enabled in mozilla-central. So, this only proofs that it's HiDPI related, which we allready knew...

Comment 10

[Markus Stange [:mstange]]

I can reproduce this problem in Firefox, too, when trying to drag history items in the Bookmarks/History library window. That's good because it means that I don't need to compile Thunderbird if I want to debug this :)

Comment 11

[Mihai]

I think I see the same issue with my Macbook Pro Retina, 13-inch, Late 2013
OS X 10.9.1 (13B3116)

What is striking is that on external monitor, it works fine. Either on Thunderbolt or on an Asus hi-res.

Comment 12

[Josiah Bruner [:jsbruner]]

Markus, will you be able to get to this soon? If not, I can take a closer look.

Comment 13

[Markus Stange [:mstange]]

Probably not, I'm afraid. Feel free to start looking :)

Comment 14

[Josiah Bruner [:jsbruner]]

Hmm. So I have a feeling that the problem may actually be with DrawDrag() (http://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/nsBaseDragService.cpp#427), since I was actually able to successfully export the file locally, where I noticed that the image size and the file size was correct. We must just be trying to draw nothing.

Comment 15

[Josiah Bruner [:jsbruner]]

So I'm doubting this is actually widget-related. The code looks okay and the function(s) actually do work properly when dragging certain things. (Such as the chat account list window). I think we instead may be calling the function incorrectly from the toolkit side.

Comment 18

[achim+github]

Just to add some information: 

* dragging up to 7 consecutive messages shows no preview
* dragging 8 or more consecutive messages generates the preview drag image, showing the top n-5 messages (which is probably why for smaller number of messages nothing or just a very thin line is shown).

In fact, the break point no preview/preview might even depend on the length of the subjects for the messages that are dragged: messages with longer subjects seem to generate the preview for a lower message count than those with shorter subjects…

Comment 20

[Mike Kaply [:mkaply]]

Just changing layout.css.devPixelsPerPx to 2.0 on a non Retina machine is enough to recreate this problem, so it's probably just a math error somewhere...

Comment 21

[Albert Scheiner [:alberts]]

Having this issue with the current 50b as well, but it appears fixed for me when testing TB53 - I tested bug 1256016. Mike, can you maybe confirm this?

Comment 22

[Mike Kaply [:mkaply]]

It does appear to be fixed, but I'd like to leave this open until I find out what fixed it so I can properly dupe.

Comment 23

[Wayne Mery (:wsmwk)]

(In reply to Mike Kaply [:mkaply] from comment #22)
> It does appear to be fixed, but I'd like to leave this open until I find out
> what fixed it so I can properly dupe.

fixed at last 20 days ago per  bug 1256016 comment 8 (assuming these have the same cause)

Comment 24

[Wayne Mery (:wsmwk)]

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #23)
> (In reply to Mike Kaply [:mkaply] from comment #22)
> > It does appear to be fixed, but I'd like to leave this open until I find out
> > what fixed it so I can properly dupe.
> 
> fixed at last 20 days ago per  bug 1256016 comment 8 (assuming these have
> the same cause)

bug 1309698 landed 2016-10-18. 
bug 1301673 landed 2016-10-19 - Fix up drag feedback image coordinates.  (Neil was a busy boy)

Comment 25

[Mike Kaply [:mkaply]]

I'm 99% site bug 1301673 fixed this so making as such.