adobe / brackets

An open source code editor for the web, written in JavaScript, HTML and CSS.
http://brackets.io
MIT License
33.25k stars 7.63k forks source link

Clicking language button the second time may accidentally select a different language. #8398

Open RaymondLim opened 10 years ago

RaymondLim commented 10 years ago
  1. Resize the height of Brackets window smaller so that the language popup list will show up with a vertical scroll bar.
  2. Open any document and then click on the upper end of the language button twice.

Result: The second click closes the language popup list, but at the same time it also selects the item from the list that is below the last one in the visible part of the list.

peterflynn commented 10 years ago

Hmm, at first I thought this was due to the margin-top: -6px that I used to adjust the vertical positioning, but it actually happens even without that. We're getting a click even on the list for pixels that aren't actually part of it... pretty weird.

peterflynn commented 10 years ago

Looks like it's caused by this Chromium bug: https://code.google.com/p/chromium/issues/detail?id=110705. Illustrated here. Basically, if an item has a box-shadow then any overflowing content will always respond to mouse events as if overflow: visible were set... even if it's not :-(

Although the bug is still open, it looks like a fix landed and might ship as soon as Chromium 36. But it would be a while before we could pick up a CEF that new...

For a workaround, we could remove the bottom part of the dropshadow (which I think looks bad anyway since it makes it seem disconnected from the status bar -- CC @larz0). Or we could add code in DropdownEventHandler to explicitly check if the click event's mouse coordinates are valid (within the bounds of the popup), discarding any erroneous events that are not...

peterflynn commented 10 years ago

Interestingly, it looks like DropdownEventHandler already includes code that tries to prevent a bug like this from happening -- see the viewOffset checks in the "mouseover.dropdownEventHandler" event handler. Maybe this was already a known issue that we worked around, but there's a slight bug in the workaround?

dangoor commented 10 years ago

@RaymondLim @peterflynn This doesn't strike me as a release blocker. What do you think?

RaymondLim commented 10 years ago

Agreed. Removing Release milestone.