856151 - Use of client coordinates in various modules need to be translated to browser coordinates

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Input, defect)

Description

[Jim Mathies [:jimm]]

We use clientX and clientY quite a bit but this is incorrect, we should be translating these according to the position of the <browser> or the deck the browsers sit in.

Comment 1

[Jim Mathies [:jimm]]

Attachment: binding cleanup

Comment 2

[Jim Mathies [:jimm]]

Attachment: binding cleanup

Comment 3

[Jim Mathies [:jimm]]

Attachment: bindings

Comment 4

[Jim Mathies [:jimm]]

Attachment: context menus

Comment 5

[Jim Mathies [:jimm]]

Attachment: bindings

I'll add more to these as I go, this is start.

Comment 6

[Jim Mathies [:jimm]]

Attachment: context menu fix

This fixes up context menu positioning when the browser position shifts. I've also added a test.

Comment 7

[Matt Brubeck (:mbrubeck)]

Comment on attachment 731877 [details] [diff] [review]
bindings

Review of attachment 731877 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/bindings/browser.xml
@@ +203,5 @@
> +
> +            return {
> +              xPos: (aBrowserX * scale - scrollX - deckX + bcr.left),
> +              yPos: (aBrowserY * scale - scrollY - deckY + bcr.top)
> +            };

Nit: I'd like to continue using {x, y} instead of {xPos, yPos}.

The main rationale is that we can then use these return values together with "Point" instances and methods from Geometry.jsm.

Comment 8

[Jim Mathies [:jimm]]

Attachment: bindings

updated per comments. I've also added some new helpers for converting individual points which I'm using in selection code.

Comment 9

[Jim Mathies [:jimm]]

Attachment: bindings

updated a couple xPos -> x type comments in the header.

Comment 10

[Frank Yan (:fryn)]

Comment on attachment 731997 [details] [diff] [review]
bindings

Review of attachment 731997 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/helperui/SelectHelperUI.js
@@ +108,5 @@
>  
>      return {
>        forcePosition: true,
> +      xPos: p0.xPos,
> +      yPos: p1.yPos,

This needs to be updated to .x and .y too.

Comment 11

[Frank Yan (:fryn)]

Comment on attachment 731878 [details] [diff] [review]
context menu fix

Review of attachment 731878 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: browser/metro/base/content/helperui/MenuUI.js
@@ +194,3 @@
>      this._menuPopup.show(Util.extend({}, this._defaultPositionOptions, {
> +      xPos: coords.xPos,
> +      yPos: coords.yPos,

This needs to be updated to .x and .y.

Comment 12

[Jim Mathies [:jimm]]

Attachment: selection

Comment 13

[Jim Mathies [:jimm]]

Attachment: selection

This passes all of my selection tests I have in my queue (amazing!).

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a5aa2d31e8e9
https://hg.mozilla.org/mozilla-central/rev/5722966a3126
https://hg.mozilla.org/mozilla-central/rev/d3a0bc6f68b1