viewZoomOverlay.js shouldn't have a wheel event listener

NEW
Assigned to

Status

SeaMonkey
UI Design
3 years ago
2 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: MikeLing)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Bug 1015721 introduces a new ZoomChangeUsingMouseWheel event which fires after the event state manager has applied the zoom thus allowing handleEvent to call _applySettingToPref directly, making _handleMouseScrolled redundant.
(Assignee)

Comment 1

2 years ago
Hi Neil,

I would like to work on this issue, could you assign it to me? Thank you!

Due to I'm still a new comer for seamonkey, though I had finished my first bug, so it will be helpful if you can tell me more about where should I start and which part of code I should looking into. :)

Comment 2

2 years ago
(from Bug 1015721 comment #43)
> https://hg.mozilla.org/mozilla-central/rev/d6100d9ff552
> https://hg.mozilla.org/mozilla-central/rev/884941dfa5c8
> https://hg.mozilla.org/mozilla-central/rev/a50a835644e3
> https://hg.mozilla.org/mozilla-central/rev/a91d65abbcf8

The second patch is the one you should look at
(Assignee)

Comment 3

2 years ago
(In reply to Philip Chee from comment #2)
> (from Bug 1015721 comment #43)

Thank you Philip, will commit a PR ASAP :)
(Assignee)

Comment 4

2 years ago
Created attachment 8658506 [details] [diff] [review]
patch_for_bug_1056299.patch

Hope this patch could help ;)
Attachment #8658506 - Flags: review?(neil)

Updated

2 years ago
Assignee: nobody → sabergeass
(Reporter)

Comment 5

2 years ago
Comment on attachment 8658506 [details] [diff] [review]
patch_for_bug_1056299.patch

As usual my description is unclear; the code needs to start using ZoomChangeUsingMouseWheel instead of wheel events.
Attachment #8658506 - Flags: review?(neil)
(Assignee)

Comment 6

2 years ago
Created attachment 8660505 [details] [diff] [review]
patch_version1.patch

Hi neil,

Could you give me some hits about why my code doesn't work at all? Thank you!

BTW, I haven't figure out how to create a patch by hg commit && hg push. Is there a way to create a patch without hg qnew && qref?
Attachment #8658506 - Attachment is obsolete: true
Attachment #8660505 - Flags: feedback?
(Reporter)

Comment 7

2 years ago
Huh, I can't even blame being half asleep this time...
(Reporter)

Comment 8

2 years ago
(In reply to comment #7)
> Huh, I can't even blame being half asleep this time...

I don't know what I was thinking then. Maybe I was half asleep?
(Reporter)

Comment 9

2 years ago
(In reply to MikeLing from comment #6)
> BTW, I haven't figure out how to create a patch by hg commit && hg push. Is
> there a way to create a patch without hg qnew && qref?

I don't think new contributors can create a patch with hg push. If you don't like mq, you may be able to use changeset evolution. At the simplest level you use hg commit to start a patch and hg commit --amend to edit it. Note that client.py doesn't really like either mq or changeset evolution; I don't know what other people do here.
(Reporter)

Comment 10

2 years ago
Comment on attachment 8660505 [details] [diff] [review]
patch_version1.patch

>-    window.addEventListener("wheel", this, true);
>+    gBrowser.addEventListener("ZoomChangeUsingMouseWheel", this);
[Not 100% sure whether gBrowser works here]

>   // nsIDOMEventListener
>-
>   handleEvent: function FullZoom_handleEvent(event) {
[Blank line got deleted by mistake]

>-    switch (event.type) {
>+      switch (event.type) {
[Wrong amount of indentation]

>+              var browser = this._getTargetedBrowser(event);
>+              this._applyPrefToSetting(browser);
This doesn't look right; I think you want to apply the setting to the pref. (And the arguments to _applyPrefToSetting are wrong anyway.)
Attachment #8660505 - Flags: feedback?

Comment 11

2 years ago
|hg help diff| has some helpful hints.
You need to log in before you can comment on or make changes to this bug.