Bug 78014 – [chromium] Pepper Plugins don't receive mouse movement information in non-mouse-lock mode.

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

WebMouseEventBuilder doesn't pass along mouse movement information properly.

Created attachment 125916 [details]
Patch

[Vincent Scheib]

LGTM, Though I'm not a reviewer.

[Tony Chang]

Is it possible to write a test for this?  Maybe we have one in chromium?

(In reply to comment #3)
> Is it possible to write a test for this?  Maybe we have one in chromium?

For now, we have a manual test at the chromium side.
(I agree that automatic tests are better. :))

[Tony Chang]

(In reply to comment #4)
> (In reply to comment #3)
> > Is it possible to write a test for this?  Maybe we have one in chromium?
> 
> For now, we have a manual test at the chromium side.
> (I agree that automatic tests are better. :))

Is it possible to write a layout test for this?

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Is it possible to write a test for this?  Maybe we have one in chromium?
> > 
> > For now, we have a manual test at the chromium side.
> > (I agree that automatic tests are better. :))
> 
> Is it possible to write a layout test for this?

I will look into it. Thanks!

[Eric Seidel (no email)]

Comment on attachment 125916 [details]
Patch

How can we test this?

[Eric Seidel (no email)]

Comment on attachment 125916 [details]
Patch

Ah, I see now that test investigation is underway.  Marking r- to remove from the queue.  Feel free to remark r? if you find testing impossible/impractical (ideally you'd update the changelog to say such).

Hi, Tony and Eric.

I took a look and think that it is probably not easy/useful to add a test for this change:
- there isn't any test for WebMouseEventBuilder right now; moreover, the movement information added is only used by pepper, and there isn't any test for pepper at the WebKit side. (AFAIK, WebKit doesn't know anything about pepper.)
- this change is trivial: it copies two newly-added data fields from MouseEvent to WebMouseEventBuilder during conversion.

Please let me know if you think it is okay to commit without a test. (And I will be happy to make changes if you know how to add a test.)

[Tony Chang]

(In reply to comment #9)
> Hi, Tony and Eric.
> 
> I took a look and think that it is probably not easy/useful to add a test for this change:
> - there isn't any test for WebMouseEventBuilder right now; moreover, the movement information added is only used by pepper, and there isn't any test for pepper at the WebKit side. (AFAIK, WebKit doesn't know anything about pepper.)
> - this change is trivial: it copies two newly-added data fields from MouseEvent to WebMouseEventBuilder during conversion.
> 
> Please let me know if you think it is okay to commit without a test. (And I will be happy to make changes if you know how to add a test.)

I think it's fine to land the change without a test.  Thanks for looking into why it doesn't make sense.  Please put the reason for no layout test in the ChangeLog.

Created attachment 126843 [details]
Patch

Thanks, Tony!

Uploaded a patch with revised ChangeLog.

[Tony Chang]

BTW, it doesn't really matter that the change is trivial.  One reason to have layout tests is to verify that there are no regressions in the future and even trivial parts of the code base can regress (e.g., when refactoring).

[WebKit Review Bot]

Comment on attachment 126843 [details]
Patch

Clearing flags on attachment: 126843

Committed r107684: <http://trac.webkit.org/changeset/107684>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.