1659232 - :-moz-drag-over selectors not working

Status: RESOLVED INVALID

(Thunderbird :: Mail Window Front End, defect)

Description

[Axel Grude]

While converting my Add-on QuickFolders for Tb78, I notice that none of my drag Over styles work.

It appears as if the:

:-moz-drag-over

pseudo selector doesn't work for me? The rules are loaded dynamically, like most of the layout stuff in this Add-on.

For testing, you can use the latest build at:
https://github.com/RealRaven2000/QuickFolders/issues/8#issuecomment-674247720

1 - drag some folders to the toolbar
2 - try to rearrange the folders. they should change color on dragover and little chevrons should appear for indicating the new position of the button.

Comment 1

[Axel Grude]

Here is a prerelease of QuickFolders (compatible with Thunderbird 78.*) for testing:

https://github.com/RealRaven2000/QuickFolders/releases/tag/5.0pre65

with this, you can easily test most dragover stuff. Drag folders to the toolbar then try exchanging their positions - dropmarkers should appear.
https://i.imgur.com/SNmBTeO.png

When dragging mail to a tab (which will move the mail to the referenced folder) the background color should change:
https://i.imgur.com/awMiXLe.png

Comment 2

[Magnus Melin [:mkmelin]]

I don't see how this is a Thunderbird bug. We don't track problems in add-ons, and we don't use that property anywhere (nor does Firefox)

Comment 3

[Axel Grude]

(In reply to Magnus Melin [:mkmelin] from comment #2)

I don't see how this is a Thunderbird bug. We don't track problems in add-ons, and we don't use that property anywhere (nor does Firefox)

Well, it's not an "Add-on bug" but obviously functionality I relied on was removed. What does Firefox / Thunderbird use for dragover styling - is there a html standard for this? If not it's a bit of a gap, don't you think? I think khushil handled some drag bugs, so I hopef he would be able to shed some light on this.

Comment 4

[Axel Grude [:realRaven]]

I have started to code workaround for my toolbar buttons (QuickFolders tabs, they represent mail folders) - see QuickFolders release 5.1. I am 100% sure that I found an article somewhere that :-moz-drag-over was removed due to "lack of take-up". (if you install QF 5.0 in Tb78 you can test this. all tabs can be re-ordered and accept mails as drop targets. The subfolder menus were also styled using this selector) Unfortunately I cannot find that article anywhere. When using inspector, this pseudo class was rejected as invalid. I thought I checked whether it was defined in m-c 78 to confirm but today I was told that it is still there, so this may actually a bug that affects Thunderbird:

According to @emilio :
"Something a bit more likely is that TB started to use shadow dom and that pseudo class is not interacting well with it or what not "

Comment 5

[Emilio Cobos Álvarez (:emilio)]

It should be able to reproduce this in an HTML page and be moved to Core. Alternatively, a bisect of when this stopped working using mozregression would be useful to know why they're not working, afaict the code implementing these hasn't changed in quite a while.

Comment 6

[Axel Grude]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

It should be able to reproduce this in an HTML page and be moved to Core. Alternatively, a bisect of when this stopped working using mozregression would be useful to know why they're not working, afaict the code implementing these hasn't changed in quite a while.

Is there any way this can be moved from RESOLVED - INVALID into something more productive? I am afraid this may never be looked at otherwise.

I just wouldn't know how to create a test case based on a HTML page unfortunately. But it should possible to create a div that accepts dragged items and then setting a rule

#myDragTarget :-moz-drag-over {
background-color: red !important;
}

I just don't know how to create draggable items / drag targets in HTML.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Test-case

The pseudo-class seems to work fine. You need to preventDefault() the dragenter event for it to apply, but again that's nothing new.

Comment 8

[Axel Grude]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

Created attachment 9199229 [details]
Test-case

The pseudo-class seems to work fine. You need to preventDefault() the dragenter event for it to apply, but again that's nothing new.

OK, I will try that - it must be new behavior as this wasn't necessary in Thunderbird 68?

Comment 9

[Axel Grude]

(In reply to Axel Grude from comment #8)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

Created attachment 9199229 [details]
Test-case

The pseudo-class seems to work fine. You need to preventDefault() the dragenter event for it to apply, but again that's nothing new.

OK, I will try that - it must be new behavior as this wasn't necessary in Thunderbird 68?

Just searching through my code, I found this line in my function addSubFoldersPopupFromList() :

menuitem.setAttribute("ondragenter","event.preventDefault();"); // fix layout issues...

so this may be an old fix for a problem, but maybe the line is syntactically wrong? Should it be "dragenter" instead?

Comment 10

[Emilio Cobos Álvarez (:emilio)]

I don't know what setEventAttribute does, but changing that line by menuitem.addEventListener("dragenter", e => e.preventDefault()) should work.

The code related to these pseudo-classes hasn't changed and it seems to be working, if anything this was a XUL change to menuitems (maybe they no longer look at ondragenter attributes, not sure, would have to bisect).

Comment 11

[Axel Grude]

(In reply to Axel Grude from comment #9)

(In reply to Axel Grude from comment #8)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

Created attachment 9199229 [details]
Test-case

The pseudo-class seems to work fine. You need to preventDefault() the dragenter event for it to apply, but again that's nothing new.

OK, I will try that - it must be new behavior as this wasn't necessary in Thunderbird 68?

Just searching through my code, I found this line in my function addSubFoldersPopupFromList() :

menuitem.setAttribute("ondragenter","event.preventDefault();"); // fix layout issues...

so this may be an old fix for a problem, but maybe the line is syntactically wrong? Should it be "dragenter" instead?

Thanks for providing the test case - I rewrote this part of code by adding the event listener like you did:

  menuitem.addEventListener("dragenter", function(e) { 
    e.preventDefault(); 
  });

and added the rules back, it appears to be working. So setting the "ondragenter" attribute like that seems to be what isn't supported anymore. Since I use this pattern quite ubiquitously in QuickFolders, what is the exact underlying issue? Is it the fact that I use event.preventDefault(); as event attribute and event isn't resolved properly? If I have to add event listeners for every time I use this pattern I am looking at some very substantial refactoring work...

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Not sure, it might be that ondragenter is not being called, or that event doesn't resolve to anything. Should be easy to check whether it's getting called. If it's being called, return false is the right thing to do rather than event.preventDefault().