[Stingray] Zoom in/out support for browser on non-touch-enabled devices

RESOLVED FIXED in 2.1 S1 (1aug)

Status

Firefox OS
Gaia::System::Browser Chrome
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: schien, Assigned: schien)

Tracking

unspecified
2.1 S1 (1aug)
Other
Gonk (Firefox OS)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

For non-touch-enabled devices, we cannot use pinch gesture to perform APZ zoom in/out while browsing web page. We need a way to trigger zoom in/out UI like Firefox browser (click button or key event).
Created attachment 8462656 [details] [diff] [review]
zoom-browser-api.patch

I tried to add a browser API for zoom in/out by leverage nsIContentViewer.fullZoom. However, the following JS error is shown while modifying fullZoom attribute in BrowserElementChildPreload.js.
>System JS : ERROR chrome://global/content/BrowserElementChildPreload.js:1036 - NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN: Cannot modify properties of a WrappedNative

Is this the right way to provide zoom in/out functionality?
Attachment #8462656 - Flags: feedback?(fabrice)
Component: General → Gaia::System::Browser Chrome
Created attachment 8462988 [details] [diff] [review]
zoom-browser-api.patch

The fullZoom attribute is recently moved from nsIMarkupDocumentViewer to nsIContentViewer and my codebase doesn't catch up with this modification. This patch works after update my codebase to latest m-c tip.
Assignee: nobody → schien
Attachment #8462656 - Attachment is obsolete: true
Attachment #8462656 - Flags: feedback?(fabrice)
Attachment #8462988 - Flags: review?(fabrice)
Created attachment 8463193 [details] [diff] [review]
gaia-example.patch

This is the sample code of how gaia can use the zoom API.
Comment on attachment 8462988 [details] [diff] [review]
zoom-browser-api.patch

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

::: dom/browser-element/BrowserElementParent.jsm
@@ +591,5 @@
>    _stop: function() {
>      this._sendAsyncMsg('stop');
>    },
>  
> +  _zoom: function(zoom) {

can you add a comment with the allowed value range for zoom?
Also, we likely want to validate the value here and/or in the child.
Attachment #8462988 - Flags: review?(fabrice) → feedback+
Created attachment 8463256 [details] [diff] [review]
zoom-browser-api.patch, v2

Gecko already defined two preferences for the valid zoom scale (zoom.minPercent/zoom.maxPercent) [1], so I use it to clip the zoom scale given in argument.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp#1955
Attachment #8462988 - Attachment is obsolete: true
Attachment #8463256 - Flags: review?(fabrice)
Attachment #8463256 - Flags: review?(fabrice) → review+
Created attachment 8463931 [details] [diff] [review]
zoom-browser-api.patch

update commit log, carry r+.

try looks good: https://tbpl.mozilla.org/?tree=Try&rev=65be9f765f3a
Attachment #8463256 - Attachment is obsolete: true
Attachment #8463931 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1727c4616c4d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S1 (1aug)

Updated

3 years ago
See Also: → bug 955959
You need to log in before you can comment on or make changes to this bug.