619937 - Clicking on empty area of Panorama makes empty group

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P2)

Description

[Alice0775 White]

Build Identifier:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101217 Firefox/4.0b9pre ID:20101217030324

Clicking on empty area of Panorama makes empty group.
Left-click/Right-crick/Middle-click on Windows
Left-click/Right-crick on Linix

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Enter Panorama (Ctrl+E)
3. Click on empty area of Panorama (without dragging)

Actual Results:
 An empty group is created

Expected Results:
 should not create any group if I click without mouse dragging

Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/075bb085d755
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101215 Firefox/4.0b9pre ID:20101216125325
Fails:
http://hg.mozilla.org/mozilla-central/rev/822e23ac1f71
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101216 Firefox/4.0b9pre ID:20101216135708
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=075bb085d755&tochange=822e23ac1f71

In local build:
Build from
822e23ac1f71 : Fails
eae26f4fac41 : Fails
7c40cabc296e : Fails
075bb085d755 : Works

Regressed by 7c40cabc296e	Michael Yoshitaka Erlewine — Bug 589206 - Drawing a tab group should be transparent or in background view [r+a=dolske]

Comment 1

[Alice0775 White]

s/Left-click/Right-crick on Linix/Left-click/Right-crick/Middle-click on  Linux/

Comment 3

[Michael Yoshitaka Erlewine [:mitcho]]

Note that this doesn't happen on beta 8.

Comment 4

[Aza Raskin [:aza]]

Nominating this as blocking; this is a pretty bad bug that users will hit all the time.

Comment 5

[Michael Yoshitaka Erlewine [:mitcho]]

Identified the revision that introduced it: it's http://hg.mozilla.org/mozilla-central/rev/7c40cabc296e, my bug 589206 patch. :(

I'm accepting this bug.

Comment 6

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Patch v1

Turns out we were determining whether a dragRegion should actually become a group or not based on its opacity value, which wasn't set if you just clicked and released. Checking actual dimensions instead.

Also added a trivial test for this. Pushed to try.

Comment 7

[Ian Gilman [:iangilman]]

Comment on attachment 500116 [details] [diff] [review]
Patch v1

Looks good. One nit:

>+	let bg = contentWindow.document.getElementById("bg");

This line has a tab character in it.

Comment 8

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Patch v1.1

Comment 9

[Michael Yoshitaka Erlewine [:mitcho]]

Pushed latest patch to try today. Passed.

Comment 10

[Kevin Hanes]

bugspam (moving b9 to b10)

Comment 11

[Kevin Hanes]

bugspam (removing b9)

Comment 12

[Michael Yoshitaka Erlewine [:mitcho]]

This bug was not in beta 8, and so if it stays in beta 9, it will be a jarring regression for beta users. We've been waiting for approval2.0 for almost a week.

Requesting blocking2.0=beta9 so that we can land this immediately.

Comment 13

[Kevin Hanes]

Comment on attachment 501075 [details] [diff] [review]
Patch v1.1

Is now blocking, no longer needs a+

Comment 14

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Patch for checkin

Comment 15

[Nochum Sossonko [:Natch]]

For the record: bug 622872 comment 5. This should help with that bug (although I'm not sure if it will help the other edge-case).

Comment 16

[Michael Yoshitaka Erlewine [:mitcho]]

Deserves a beta 9 relnote.

Comment 17

[Ian Gilman [:iangilman]]

http://hg.mozilla.org/mozilla-central/rev/00893dfbaedb

Comment 18

[Tracy Walker [:tracy]]

verified with nightly build of minefield.