Closed Bug 1044066 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox OS Graveyard :: Gaia::System::Browser Chrome, defect)

Other
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.1 S1 (1aug)

People

(Reporter: schien, Assigned: schien)

References

Details

Attachments

(2 files, 3 obsolete files)

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).
Attached patch zoom-browser-api.patch (obsolete) — Splinter Review
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
Attached patch zoom-browser-api.patch (obsolete) — Splinter Review
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)
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+
Attached patch zoom-browser-api.patch, v2 (obsolete) — Splinter Review
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+
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+
https://hg.mozilla.org/mozilla-central/rev/1727c4616c4d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S1 (1aug)
See Also: → 955959
You need to log in before you can comment on or make changes to this bug.