Closed Bug 784887 Opened 7 years ago Closed 7 years ago

Add message passing for pinch-to-zoom to JS

Categories

(Firefox for Android :: Toolbar, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 18

People

(Reporter: jwir3, Assigned: jwir3)

References

Details

Attachments

(1 file, 2 obsolete files)

According to mbrubeck, we currently can't get the coordinates of a pinch-zoom event in javascript. We'll need to support Java->JS messages for pinch to zoom events for the reflow-on-zoom effort being developed as part of Readability 2.0.
I would like to avoid adding a special message for this.

I think I'd rather we just dispatch correct MozMagnifyGesture events. Right now we DO send them from nsWindow.cpp, but I don't know if they're correct or not (XUL Fennec was using them an no one complained, so I think they're fine). Since they're using our own gesture code, they probably also don't match up with what we're getting in Java.
(In reply to Wesley Johnston (:wesj) from comment #1)
> I would like to avoid adding a special message for this.
> 
> I think I'd rather we just dispatch correct MozMagnifyGesture events. Right
> now we DO send them from nsWindow.cpp, but I don't know if they're correct
> or not (XUL Fennec was using them an no one complained, so I think they're
> fine). Since they're using our own gesture code, they probably also don't
> match up with what we're getting in Java.

Ok, I can do that instead, but why do you want to avoid a special message for pinch events? (Perhaps I'm not understanding correctly, but it seems that doing this particular event in a special message-passing way is inconsistent with how we do message passing for other gesture events, e.g. double- and single- tap).
Attached patch b784887 (WIP) (obsolete) — Splinter Review
I'm not sure if you or Wes are the correct person(s) to review this patch, but I thought I would post it as-is right now. This is in support of reflow-on-zoom for pinch zooming.
Attachment #663718 - Flags: review?(mbrubeck)
(In reply to Scott Johnson (:jwir3) from comment #2)
> (In reply to Wesley Johnston (:wesj) from comment #1)
> Ok, I can do that instead, but why do you want to avoid a special message
> for pinch events? (Perhaps I'm not understanding correctly, but it seems
> that doing this particular event in a special message-passing way is
> inconsistent with how we do message passing for other gesture events, e.g.
> double- and single- tap).

I'm fine with doing things this way for now, but I'm planning to move our singletap, doubletap, and this code into platform eventually instead of using JS message passing. We/I did that originally because we needed a way to "fluff" out the event targets (for single tap especially). That's been moved into platform now, and I would rather use HTML "standard" methods than the ones we made up. Doubletap events I just forgot about. 

Quirksmode has mentioned before that they'd like a "zoom" event so that pages can know what is happening. But there's no good spec for how this stuff should work:

http://www.quirksmode.org/dom/events/resize_mobile.html
Attached patch b784887 v2 (WIP) (obsolete) — Splinter Review
Accidentally uploaded the wrong patch.
Attachment #663718 - Attachment is obsolete: true
Attachment #663718 - Flags: review?(mbrubeck)
Attachment #663726 - Flags: review?(mbrubeck)
Attachment #663726 - Attachment is patch: true
Attachment #663726 - Flags: review?(mbrubeck) → review?(wjohnston)
Attached patch PatchSplinter Review
Sorry to inundate you with patches brad. If there's someone else who can review this (kats is out?), I can send it to them.

I think I'd rather do something like this. This kills off our current gesture dispatching, which I'm not sure worked. It replaces it with (for now) just MozMagnifyGesture events sent ONLY when we're actually magnifying the page, and based on what we get in our Java pinch zoom detector. We can add back swipe later if we want/need it.

You should be able to listen for these by attaching a listener to the browser.xul document. i.e.

document.addEventListener("MozMagnifyGestureStart", this, false);
document.addEventListener("MozMagnifyGesture", this, false);
document.addEventListener("MozMagnifyGestureUpdate", this, false);
Attachment #666675 - Flags: review?(blassey.bugs)
Attachment #663726 - Attachment is obsolete: true
Attachment #663726 - Flags: review?(wjohnston)
Attachment #666675 - Flags: review?(blassey.bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/f1646acf98b3
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Depends on: 797705
You need to log in before you can comment on or make changes to this bug.