Closed Bug 886573 Opened 7 years ago Closed 6 years ago

Keyboard zoom controls change shown percentage in Australis zoom widget while in customize mode

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 30
Tracking Status
firefox29 --- verified
firefox30 --- verified

People

(Reporter: Dolske, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P5][good first bug][mentor=jaws][lang=js])

Attachments

(2 files)

Seen on user testing. Using the keyboard controls to zoom in/out while in customize mode thankfully doesn't actually zoom the page, but does make the zoom percentage label change in the widget.
To fix this bug, you should clone the UX branch which is located at https://hg.mozilla.org/projects/ux/. You will need to look at the /browser/components/customizableui/src/CustomizableWidgets.jsm and probably need to update the zoom-controls widget to disregard zoom notification changes while in customization mode.
Whiteboard: [Australis:M?][Australis:P5] → [Australis:M?][Australis:P5][good first bug][mentor=jaws][lang=js]
Hi, 

I am interested in working on this bug. So please can you assign this bug to me.

Thanks in advance,

Regards,
A.Anup
Assignee: nobody → allamsetty.anup
Assignee: allamsetty.anup → darshanapadmadas
Attached patch output.patchSplinter Review
Made the necessary changes.
Attachment #8363005 - Flags: review?(jaws)
Comment on attachment 8363005 [details] [diff] [review]
output.patch

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

::: browser/components/customizableui/src/CustomizableWidgets.jsm
@@ +363,5 @@
>        let window = aDocument.defaultView;
>        function updateZoomResetButton() {
>          //XXXgijs in some tests we get called very early, and there's no docShell on the
>          // tabbrowser. This breaks the zoom toolkit code (see bug 897410). Don't let that happen:
> +        if (document.documentElement.hasAttribute("customizing"))

`document` is not defined here. How did you test this?
Attachment #8363005 - Flags: review?(jaws) → review-
Duplicate of this bug: 979011
Darshana, I'm sorry, but I'm stealing this because it annoys me so much and it gets duplicated because it confuses people. I ended up adding some more bits because otherwise the display wasn't getting updated at the right times, because the transition takes a bit to finish, yada yada.
Attachment #8385624 - Flags: review?(jaws)
Assignee: darshanapadmadas → gijskruitbosch+bugs
Comment on attachment 8385624 [details] [diff] [review]
make zoom controls display the right thing in Australis even in customize mode,

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

This looks good, thanks!
Attachment #8385624 - Flags: review?(jaws) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/1cec9e21e02b
Whiteboard: [Australis:M?][Australis:P5][good first bug][mentor=jaws][lang=js] → [Australis:P5][good first bug][mentor=jaws][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/1cec9e21e02b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P5][good first bug][mentor=jaws][lang=js][fixed-in-fx-team] → [Australis:P5][good first bug][mentor=jaws][lang=js]
Target Milestone: --- → Firefox 30
Comment on attachment 8385624 [details] [diff] [review]
make zoom controls display the right thing in Australis even in customize mode,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis
User impact if declined: zoom reset button has confusing label (and has a label which can change) when in customize mode
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none
Attachment #8385624 - Flags: approval-mozilla-aurora?
Attachment #8385624 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.