Closed Bug 524479 Opened 10 years ago Closed 10 years ago

Secondary zoom mechanism works when in Preferences

Categories

(Firefox for Android Graveyard :: Panning/Zooming, defect, major)

Fennec 1.1
defect
Not set
major

Tracking

(fennec1.0+)

VERIFIED FIXED
fennec1.0
Tracking Status
fennec 1.0+ ---

People

(Reporter: aakashd, Assigned: mfinkle)

Details

Attachments

(1 file)

Build Id:

Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20091026
Fennec/1.0b5pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20091026
Fennec/1.0b5pre


Steps to Reproduce:
1. Go to the Preferences Pane
2. Use the secondary zoom mechanism (Ctrl+Up/Down)
3. Go back to browser window

Actual Results:
The content window in the browser pane is zoomed in

Expected Results:
The zoom mechanism should be disabled when the user switches out of the browser pane.
tracking-fennec: --- → ?
Flags: in-litmus?
We should probably have a broadcaster that keeps track of whether the content is visible, and have the zoom keys (and others?) observe it.
tracking-fennec: ? → 1.0+
Attached patch patchSplinter Review
This patch adds a broadcasterset and a broadcaster to monitor the visibility of the content area. If a full screen dialog or the tools panel obscures the content, the broadcaster is disabled. When the dialog or panel is hidden, the broadcaster is enabled again.

Currently, the zoom commands observe the broadcaster and will enable/disabled with it.
Assignee: nobody → mark.finkle
Attachment #413682 - Flags: review?(combee)
Comment on attachment 413682 [details] [diff] [review]
patch

It gets my OK with one question -- why change the IDs to remove "main"?  That seems like an unrelated change and something that could break extensions.

Also, there's a small whitespace issue with adding the code in popDialog.
Attachment #413682 - Flags: review?(combee) → review+
Agree with Ben about the ID changes, they seem gratuitous. "cast_contentShowing" is also a rather weird ID... how about just "contentShowing"? Could also add it to the Elements object (bug 529935).
(In reply to comment #4)
> Agree with Ben about the ID changes, they seem gratuitous.

Yeah, I knew that when I made the changes, but my OCD was kicking in, since the IDs are not what we typically use. Sign, I guess I can live with it.

> "cast_contentShowing" is also a rather weird ID... how about just
> "contentShowing"? Could also add it to the Elements object (bug 529935).

"contentShowing" seemed like an easy conflict so I prefixed. I can make it "bcast_contentShowing" and I'll add to Elements too.
pushed with tweaks:
https://hg.mozilla.org/mobile-browser/rev/808a5914bc9a
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Post-B5
verified FIXED on build:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2b4pre) Gecko/20091124 Firefox/3.6b4pre Fennec/1.0b6pre

and

Mozilla/5.0 (X11; U; Linux armv6l; Nokia N8xx; en-US; rv:1.9.3a1pre) Gecko/20091124 Firefox/3.7a1pre Fennec/1.0b5
Status: RESOLVED → VERIFIED
litmus testcase 9796 has been created to regression test this bug.
Flags: in-litmus? → in-litmus+
Component: General → Panning/Zooming
You need to log in before you can comment on or make changes to this bug.