1257815 - (DOMMouseScroll-vs-webdevs) [meta] DOMMouseScroll event should never fire ±32768 (should fire ±3 insted), because nobody knows about that, even Mozilla's tutorials and developers.

Status: REOPENED

(Core :: DOM: Events, defect, P5)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 46, 32bit, ID 20160209030347
Read dependant bugs, really. Especially bug 1247509, bug 1253484

STR:
1. Set mouse option "when I rotate mouse wheel" in your OS to "scroll by 1 page"
2. Find some sites that rely on ev.detail
3. Try to use such site

Result:       
 Every such site is broken, because Mozilla decided to break UX for users who decided to
 scroll document by 1 page, by returning ev.detail == ±32768 instead of ±3
 Web devs are unaware of that, because that only happens for a small part of users. So
 Mozilla basically set a trap for those users by introducing a malicious API. It was
 obvious from the start that web devs won't handle such strange API

Expectations: 
 UX shouldn't be intentionally broken by Mozilla. ev.detail should return ±3

Note:
 It works fine in IE11 and Chrome so this is firefox bug. Read dependant bugs.

Comment 1

[arni2033]

It's not worth a separate bug report, but I encountered the same bug on this site:
http://cheatspirates.com/swordsman-hack-free-gold-and-sycee-cheats/ (from bug 1197601)

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't think that we should change our behavior of DOMMouseScroll anymore because changing the behavior may break some websites which tested the behavior *enough*.

Currently, whole major browsers already support "wheel" events. WheelEvent has deltaMode which indicates its delta(X|Y) unit. So, anyway, broken websites should be updated for newer browsers.

Comment 3

[arni2033]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #2)
> I don't think that we should change our behavior of DOMMouseScroll anymore because
> changing the behavior may break some websites which tested the behavior *enough*.
You've already broken UX for years fot this category of users, why are you acting so carefully now?
Please show me one site that relies on ev.detail and works correctly. Site that special-cased 32768.
What sites exactly did you mean? I've provided many arguments and seen none from your side.

> Currently, whole major browsers already support "wheel" events. WheelEvent has deltaMode which 
> indicates its delta(X|Y) unit. So, anyway, broken websites should be updated for newer browsers.
1) They're only broken on Firefox, because it originally introduced incomplete API (DOMMouseScroll).
2) If you wanted them to "be updated for newer browsers" (which is wrong term, because there're
   sites that use DOMMouseScroll and work on newer versions) you would deprecte DOMMouseScroll.
3) What you're suggesting now is for each web dev to spend 15 minutes instead of fixing the original
   issue in 15 minutes. And use logical failures as an argument  ._. (I'd like it if you wouldn't)

I already explained (bug 1253484) that API which (on 0.1% machines) stores a random huge string in a property that is usually used for storing ±3 (and your tutorials rely on that!) - IS a trap.
[Even?] Mozilla devs were caught in this trap (bug 1171659, bug 1239534, bug 1251987)
Another explanation (bug 1247509 comment 4) shows that the change I suggest can only fix stuff:
it was useless for web devs to use Mozilla's special values when other browsers didn't have them.
'wheel' was created after a while, when all reported bugs were already presented.

I noticed that recently Mozilla only does _bad_ stuff to users from "0.1%"-category, but isn't it enough already? It's been _five_years_ of these DOMMouseScroll bugs. Maybe it's time to finish this?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

We've experienced a lot of cases which we fixed "bugs" which were not defined by any standards causing new regressions on some other web sites. In other words, touching the behavior of non-standardized feature is very risky. So, if we would try to fix this, we would need to find a way of minimizing the risk.

Comment 5

[arni2033]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #4)
> So, if we would try to fix this, we would need to find a way of minimizing the risk.
If you say so... But for 0.1% who scroll by 1 page, things are already in bad condition, AND the suggested change affects only those users. In other words, you can't break it more.
Actually, it doesn't look like those users noticed any bug. And I suppose they wouldn't notice if you changed something  v_v

The argument below seems logical, but if everybody's so cautious, I'll find a workaround myself.

In my view, the history of mouse events is:  «For the long time,
(1) there were only 'DOMMouseScroll' vs 'mousewheel', and then
(2) 'wheel' was implemented almost at the same time on all browsers.
During time period (1) it was pointless to check the type of scrolling, because most of browsers
(>50% back then?) used 'mousewheel'. So web devs took the value e.detail*40||e.wheelDelta and used it
independantly of event type. The possibility to check type of scrolling only really appeared in (2)»

So we can expect ALL sites that want to know the type of scrolling to use 'wheel'.
And the risk seems minimal as is. We can expect that suggested change (A) won't break sites that want
to know the type of scrolling, (B) won't break other sites, because it could worsen UX for 0.1% who
scroll by 1 page only if a site is already broken for everybody else. That's clearly site's fault

// I also suggested extension as an option, but couldn't make it (bug 1247509 comment 4)

Comment 6

[arni2033]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #2)
> So, anyway, broken websites should be updated for newer browsers.
I must set NI now, because you obviously haven't noticed that a lot of internal Firefox code is affected by this bug too. If you did know that while writing that phrase about websites, then it's clearly a logical failure: you were trying to make it look like it's only some broken sites, not the browser. But the browser itself is affected: PDFjs, Codemirror, etc. I call that "chain bugs"

If you don't believe me, don't believe web devs, then probably you'll believe the people who integrated all that broken stuff in Firefox. They are unaware of 32768.

Comment 7

[arni2033]

Witness #1. The person who implemented bug 1171659 in pdfjs

Hello, Jakob Miland! I found your commit to pdfjs that didn't change much in time:
https://github.com/mozilla/pdf.js/blame/45810e0866e4d2047e71501b03ff92051cb1b2ad/web/viewer.js#L1786
-> https://github.com/mozilla/pdf.js/commit/2dacbb7a03f591218b0b0fdd83765ebe18405aea
So it was you who implemented wrong scroll handling in PDF.js (read bug 1171659).
Please answer my questions:
1) Did you forget about 32768 because it's really strange and no tutorials ever mentioned it?
2) Do you blame creators of DOMMouseScroll event (as I do) or admit that it's your own fault?
3) Would your code work nice if 32768 in their code was replaced with 3?

Comment 8

[arni2033]

Witness #2. The person who repeatedly tested and edited the code causing bug 1171659

Hello, Jonas Jenwald! I found your commits to pdfjs which canged one wrong handling to another:
https://github.com/mozilla/pdf.js/commit/95b2ec124bb8bdcfabfc69e377b7d28e9dcab569
https://github.com/mozilla/pdf.js/commit/a1f4bff4f38b92df34ede30a5cbb3f54f39ee5ad
Please answer my questions:
1) Did you forget about 32768 because it's really strange and no tutorials ever mentioned it?
2) Do you blame creators of DOMMouseScroll event (as I do) or admit that it's your own fault?
3) Would your code work nice if 32768 in their code was replaced with 3?

Comment 9

[arni2033]

Witness #3. The person who integrated pdfjs (bug 714712). Also, the person who was really surprised
when I came to #pdfjs and told everybody about that strange "±32768" values. They all were surprised

Hello, Yury Delendik! You've integrated pdfjs and apparently tested it "enough" as said in comment 2
Please answer my questions:
1) Did you forget about 32768 because it's really strange and no tutorials ever mentioned it?
2) Do you blame creators of DOMMouseScroll event (as I do) or admit that it's your own fault?
3) Would the component you integrated work nice if 32768 in their code was replaced with 3?

Comment 10

[Yury Delendik (:yury)]

(In reply to arni2033 from comment #9)
> Witness #3. The person who integrated pdfjs (bug 714712). Also, the person
> who was really surprised
> when I came to #pdfjs and told everybody about that strange "±32768" values.
> They all were surprised
> 
> Hello, Yury Delendik! You've integrated pdfjs and apparently tested it
> "enough" as said in comment 2
> Please answer my questions:
> 1) Did you forget about 32768 because it's really strange and no tutorials
> ever mentioned it?

I would say "yes". However even not looking at DOMMouseScroll in depth, we would not expect to see 32768 there.

> 2) Do you blame creators of DOMMouseScroll event (as I do) or admit that
> it's your own fault?

Hard to tell. We were/are looking at replacing DOMMouseScroll with mousewheel. And the former is/will be used to polyfill (?). So we did not expect DOMMouseScroll will produce something else that we need to process differently.


> 3) Would the component you integrated work nice if 32768 in their code was
> replaced with 3?

Maybe. But we are looking at "mousewheel" event support. See https://github.com/mozilla/pdf.js/pull/4786

Comment 11

[Masatoshi Kimura [:emk]]

The "wheel" event is different from the "mousewheel" event, by the way. (Yet another proof that people don't read specs...)

Comment 12

[Yury Delendik (:yury)]

(In reply to Masatoshi Kimura [:emk] from comment #11)
> The "wheel" event is different from the "mousewheel" event, by the way. (Yet
> another proof that people don't read specs...)

In PDF.js, we needed "wheel" event when it was not available. DOMMouseScroll was used just to fill the gap.

Comment 13

[arni2033]

Witness #4. The person who touched the code regarding scrolling scrollboxes using mouse wheel
chrome://global/content/bindings/scrollbox.xml (search for "DOMMouseScroll")

Hello, Frank Yan! I found your patch to Firefox which canged one wrong handling to another:
http://hg.mozilla.org/mozilla-central/rev/f572a9ab898e (bug 565783) VS my bug 1251987, bug 1251980
Please answer my questions if you have a few minutes:
1) Did you forget about 32768 because it's really strange and no tutorials ever mentioned it?
2) Do you blame creators of DOMMouseScroll event (as I do) or admit that it's your own fault?
3) Would your code work nice if 32768 in their code was replaced with 3?

Comment 14

[arni2033]

Such a shame that I couldn't find a person who implemented scrollbox.xml. And I couldn't access
Robert O'Callahan! He was somewhere around a week ago! I guess he did read specs.
He also touched the code regarding scrolling scrollboxes using mouse wheel
chrome://global/content/bindings/scrollbox.xml (search for "DOMMouseScroll")

I found his patch to Firefox Core that canged one wrong handling to another:
http://hg.mozilla.org/mozilla-central/rev/a02e6af74ab8 (bug 378028) VS my bug 1251987, bug 1251980

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

arni2033, it would be better to file separate bugs for each issue you find in Firefox code, since that is how they will be fixed anyhow, one by one.

Comment 16

[arni2033]

(In reply to Olli Pettay [:smaug] from comment #15)
> arni2033, it would be better to file separate bugs for each issue you find in Firefox code
And so I did. But once I understood that attempt to fix all the sites and Firefox Core bugs is pointless, I decided to have a serious discussion on the underlying problem. The "±32768 issue", this
bug. You can see all issues listed in dependant bugs, because changing 32768 to 3 will fix them all.

@ Yury Delendik: Thanks for caring about the user (no irony). Sadly, I bookmarked that pull request long time ago. And I filed this bug only to prove my point that nobody expects 32768 in event.detail

Comment 17

[arni2033]

Witness #5. The person who implemented Tilt in Firefox devtools. Tilt was deleted recently =(   but
the fact stays the same: that person was an experienced developer but still haven't heard of 32768.

Hello, Victor Porof! I found bug 659807 where you implemented Tilt in Firefox
BUT, in function "mouseScroll" you used e.detail of DOMMouseScroll event:
https://github.com/victorporof/Tilt/blob/master/src/chrome/content/TiltChromeController.js#L261
I use mouse option "scroll by 1 page" whole life, so when I tried to zoom with mouse wheel in Tilt,
e.detail was equal to 32768, and just maximized/minimized the view, like described in dependant bugs.

Please answer my questions if you have a few minutes:
1) Did you forget about 32768 because it's really strange and no tutorials ever mentioned it?
2) Do you blame creators of DOMMouseScroll event (as I do) or admit that it's your own fault?
3) Would your code work nice if 32768 in their code was replaced with 3?

Comment 18

[Victor Porof [:vporof][:vp]]

Can't work on this right now, busy with Tofino.

Comment 19

[arni2033]

The last argument, in fact. No more "spamming".

Witness #6 [LAST]. The person who (mostly?) created CodeMirror!

Hello, Marijn Haverbeke! I found your commit(s) to CodeMirror which implemented scrolling
https://github.com/codemirror/CodeMirror/blame/master/lib/codemirror.js#L4003
BUT, in function "onScrollWheel" you determined variable "dy" as e.detail of DOMMouseScroll event:
https://github.com/codemirror/CodeMirror/blob/master/lib/codemirror.js#L4003
I use mouse option "scroll by 1 page" whole life, so when I tried to scroll with mouse wheel in
CodeMirror, "dy" was equal to 32768, and it caused function "updateDisplaySimple" to update massive
amount of lines. Which caused bug 1239534. Note that now with APZ and stuff smooth scrolling is
impossible, and this issue has magically gone away (for a while?). It was bad and quite long-standing
But the fact stays the same: 32768 was an unexpected value for you.

Please answer my questions if you have a few minutes:
1) Did you forget about 32768 because it's really strange and no tutorials ever mentioned it?
2) Do you blame creators of DOMMouseScroll event (as I do) or admit that it's your own fault?
3) Would your code work nice if 32768 in their code was replaced with 3?   [I just checked. It would]

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to arni2033 from comment #6)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #2)
> > So, anyway, broken websites should be updated for newer browsers.
> I must set NI now, because you obviously haven't noticed that a lot of
> internal Firefox code is affected by this bug too. If you did know that
> while writing that phrase about websites, then it's clearly a logical
> failure: you were trying to make it look like it's only some broken sites,
> not the browser. But the browser itself is affected: PDFjs, Codemirror, etc.
> I call that "chain bugs"

Each bug in our code should be fixed with using wheel event.

> If you don't believe me, don't believe web devs, then probably you'll
> believe the people who integrated all that broken stuff in Firefox. They are
> unaware of 32768.

I'm not sure what did you mean with this words.


Anyway, I don't think that we should change the legacy behavior anymore because all current browsers already support new wheel events. So, there is no reason to take risk for legacy events.

Additionally, I checked our document of DOMMouseScroll. Then, *I* wrote about the delta value of page scroll at the first revision of the document.
https://developer.mozilla.org/en-US/docs/Web/Events/DOMMouseScroll
So, it doesn't make sense to claim "nobody knows about this". Even though it were true, we had already done what we should do. So, changing the delta value as you suggested may cause break some web sites which have read our document properly. So, I don't think that "hiding" (not "fixing") existing bugs with breaking correct code is really wrong approach.

Comment 21

[arni2033]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #20)
> Each bug in our code should be fixed with using wheel event.
I don't see any work on those bugs. People just tend to remove NI and wait several decades until yet another component of Firefox is deleted. That's what happened with Panorama bugs.

> > If you don't believe me, don't believe web devs, then probably you'll
> > believe the people who integrated all that broken stuff in Firefox. They are
> > unaware of 32768.
> I'm not sure what did you mean with this words.
Everybody who ever used DOMMouseScroll is unaware of 32768. Is that clear now what I said?

> Additionally, I checked our document of DOMMouseScroll. Then, *I* wrote 
> about the delta value of page scroll at the first revision of the document.
> https://developer.mozilla.org/en-US/docs/Web/Events/DOMMouseScroll
> So, it doesn't make sense to claim "nobody knows about this". Even though it
> were true, we had already done what we should do.
Yes, you've written document that doesn't mention 32768, so everybody proceeds exactly like that:
https://developer.mozilla.org/en-US/docs/Web/Events/wheel
Web devs don't have to know which one was written by *you* specifically and therefore "more reliable"

> So, I don't think that "hiding" (not "fixing") existing
> bugs with breaking correct code is really wrong approach.
The original bug is DOMMouseScroll reporting wrong value so this is the root of the problem.

> So, changing the delta value as you suggested may cause break some web sites
This is not the only thing I suggested for you to correct your long-standing mistake.
Also, you still haven't shown a single site that is aware of 32768. While I just took every occurence of usage DOMMouseScroll in (sic!) FIREFOX and it appeared to be wrong.

Comment 22

[Dook]

maybe depends on: https://bugzilla.mozilla.org/show_bug.cgi?id=1272991
because:
"I've tested again, and I realize that the element type <arrowscrollbox> is the criminal who don't respect "one page scroll" feature.

    http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/scrollbox.xml#469
    http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/scrollbox.xml#303

When you move the wheel, DOMMouseScroll event is fired. The event has a property named "detail", and the value indicates how many lines we should scroll in the box. On <tree> or <listbox>, "event.detail" says correct value. However, on an <arrowscrollbox> element, when I set "mousewheel.withnokey.action" to "1", then "event.datail" is "2" on my environment. After I set "mousewheel.enable_pixel_scrolling" to "false", then "event.datail" is "32768" (equals to 215 - this is quite odd!) Both values are wrongly generated by Firefox itself. Now I'm reserching why the "event.detail" became an invalid value "32768"".

Comment 23

[James Willcox (:snorp) ([e-mail]) (he/him)]

Andrew what's going on with this one? We ran into it during triage. Seems important.

Comment 24

[Andrew Overholt [:overholt]]

smaug, what should we do here? We have bug 1266750 related to telemetry here.