Opened 4 years ago

Closed 4 years ago

#2993 closed bug (fixed)

box handler cancels click

Reported by: pgiraud Owned by: pgiraud
Priority: blocker Milestone: 2.11 Release
Component: Handler.Box Version: 2.10
Keywords: Cc:
State: Commit

Description

Since the revision 10871, the box handler prevents the click events from being triggered.

As said almost everywhere "The click event occurs when the pointing device button is clicked over an element. A click is defined as a mousedown and mouseup over the same screen location.". This means that "mousedown" and "mouseup" have to occur on the same element.

In our case, when "mousedown" is triggered, the box handler creates a new div element which is appended under the mouse cursor. When "mouseup" is triggered, the event target is not the same. The "click" event is not triggered.

Before the changeset, the box was positioned with a tiny offset. Now that positioning is better, the box div is exactly under the mouse.

A straight way to solve the issue is to position the box under the cursor only once the latter is actually moved.

Attached is a patch, and an example to manually test the issue.

Attachments (3)

patch-2993-A0.diff (676 bytes) - added by pgiraud 4 years ago.
patch for Handler.Box
click_and_box.html (3.9 KB) - added by pgiraud 4 years ago.
Manual test example
patch-2993-A1.diff (611 bytes) - added by pgiraud 4 years ago.

Download all attachments as: .zip

Change History (9)

Changed 4 years ago by pgiraud

patch for Handler.Box

Changed 4 years ago by pgiraud

Manual test example

comment:1 Changed 4 years ago by erilem

  • State changed from Review to Commit

Confirming that tests pass in Chrome 8 and FireFox 3. Good catch. Good patch. Please commit.

comment:2 Changed 4 years ago by pgiraud

  • Resolution set to fixed
  • Status changed from new to closed

Fixed in r10996.

comment:3 Changed 4 years ago by pgiraud

  • Priority changed from major to blocker
  • Resolution fixed deleted
  • State Commit deleted
  • Status changed from closed to reopened

There's a regression in r11749.

Changed 4 years ago by pgiraud

comment:4 Changed 4 years ago by pgiraud

  • State set to Review

comment:5 Changed 4 years ago by erilem

  • State changed from Review to Commit

Thanks for the patch Pierre. It looks good. Please commit.

comment:6 Changed 4 years ago by pgiraud

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [11923]) Don't put the box under the cursor until the mouse moved, prevent errors with the GetFeature and click events. I also removed the no longer valid (and failing) tests. r=elemoine, (Closes #2993)

Note: See TracTickets for help on using tickets.