Closed Bug 946970 Opened 11 years ago Closed 8 years ago

Add support for full zoom via toolkit's ZoomManager

Categories

(Firefox for Metro Graveyard :: Pan and Zoom, defect, P1)

x86_64
Windows 8
defect

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: smaug, Unassigned)

References

()

Details

(Whiteboard: p=5)

Attachments

(1 file, 1 obsolete file)

Currently zoom doesn't seem to work at all in Metrofox.
ctrl+/- or pinch gesture from touchpad don't do anything
This is expected. We've disabled the old toolkit zoom. To get this working we'll need to hook all the older input shortcuts and up to the apzc, which unfortunately we won't have for fx28.
(In reply to Jim Mathies [:jimm] from comment #1)
> This is expected. We've disabled the old toolkit zoom. To get this working
> we'll need to hook all the older input shortcuts and up to the apzc, which
> unfortunately we won't have for fx28.

Actually, we could just make the keyboard/trackpad/wheel zoom use fullZoom as in desktop Firefox.  This is what IE and Chrome do, both on the desktop and in Metro:  APZC-style viewport zooming for touch, and desktop-style layout zooming for keyboard/trackpad/wheel.
(In reply to Matt Brubeck (:mbrubeck) from comment #2)
> (In reply to Jim Mathies [:jimm] from comment #1)
> > This is expected. We've disabled the old toolkit zoom. To get this working
> > we'll need to hook all the older input shortcuts and up to the apzc, which
> > unfortunately we won't have for fx28.
> 
> Actually, we could just make the keyboard/trackpad/wheel zoom use fullZoom
> as in desktop Firefox.  This is what IE and Chrome do, both on the desktop
> and in Metro:  APZC-style viewport zooming for touch, and desktop-style
> layout zooming for keyboard/trackpad/wheel.

Didn't we disable page zoom because it was in conflict with apzc zoom? I can't remember the bug number.
(In reply to Jim Mathies [:jimm] from comment #3)
> Didn't we disable page zoom because it was in conflict with apzc zoom? I
> can't remember the bug number.

We removed the old Control-Plus/Minus shortcuts which used the "AnimatedZoom" class which was how we did viewport zooming before APZC.  We can't just restore that old code since it won't work in the APZC world, as you say.  We'd add new code that used ZoomManager instead (like desktop Firefox).
Too late to block28 on this?
Whiteboard: [triage]
Summary: Zoom is disabled in Metrofox → Page zoom is currently disabled in Metro Firefox
Blocks: 948328
Blocks: metrov1backlog
No longer blocks: metrobacklog
Blocks: metrobacklog
No longer blocks: metrov1backlog
Whiteboard: [triage]
Whiteboard: [v1?]
Whiteboard: [v1?] → [feature] p=0
Priority: -- → P1
Whiteboard: [feature] p=0 → p=0
Assignee: nobody → jmathies
Whiteboard: p=0 → p=5
Attached patch wip v.1 (obsolete) — Splinter Review
This works great, except that apzc panning experiences heavy checkerboarding (black rects at the bounds) when zoom is applied.
You can also apzc zoom pages out that have been zoomed in, and sort of lock them into the center of the screen heavily cropped.
Although we've tried to write the APZ code so that it works with full zoom, it's quite possible that there are issues there because until now we've never been able to test it.
Depends on: 980906
Summary: Page zoom is currently disabled in Metro Firefox → Add support for font inflation via toolkit's ZoomManager
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #10)
> Although we've tried to write the APZ code so that it works with full zoom,
> it's quite possible that there are issues there because until now we've
> never been able to test it.

now we do! :)
I'm using the wrong terminology I guess.
Summary: Add support for font inflation via toolkit's ZoomManager → Add support for full zoom via toolkit's ZoomManager
Depends on: 980923
Depends on: 980925
Depends on: 980926
Depends on: 980931
Attached patch wip v.2Splinter Review
- added keyboard shortcuts
Attachment #8387568 - Attachment is obsolete: true
Hmm, strange, text selection with the mouse has offset coordinates. I'd expect problems with our ui widget positioning, but not with platform code.
(In reply to Jim Mathies [:jimm] from comment #14)
> Hmm, strange, text selection with the mouse has offset coordinates. I'd
> expect problems with our ui widget positioning, but not with platform code.

Tracked this down, we need to override the fullZoom prop in our bindings so it points to the right nsIMarkupDocumentViewer.
Assignee: jmathies → nobody
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: