replaced e.which, that was always 1 alteast on firefox

[w3sami]

it seems e.which is depricated
https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/which

[pwambach]

yes it works now on firefox and chrome. but now it's broken in safari :(
which os do you use?

[w3sami]

Ubuntu primarily, I do have ipad with safari. I Googled this and it seems
safari doesn’t have e.buttons so maybe test it !=defined first and use the
old which in that case. Im just going to sleep cet+2 here in finland
On Oct 29, 2015 10:11 PM, "Philipp Wambach" notifications@github.com
wrote:

yes it works now on firefox and chrome. but now it's broken in safari :(
which os do you use?

—
Reply to this email directly or view it on GitHub
#20 (comment)
.

[pwambach]

I made a small test jsbin and the result is as you said: buttons is undefined in Safari.
This is the behaviour on OSX when the left mouse button is pressed during mousemove:

	Chrome	Firefox	Safari
e.button	0	0	0
e.buttons	1	1	undefined
e.which	1	1	1

and when the button is not pressed (note that on firefox which is still 1):

	Chrome	Firefox	Safari
e.button	0	0	0
e.buttons	0	0	undefined
e.which	0	1	0

so as you said we have to check if e.which is undefined and use e.buttons instead
var buttonPressed = (e.which === undefined) ? e.which : e.buttons

if(e.buttons === undefined) {
 //Safari -> use e.which
} else {
 //others -> use e.buttons
}

could you check it on Linux? I think IE could be another problem...

[w3sami]

Hi,

tested with linux and all good. Used browserstack to test with ie 10 and
it was ok.

sadly, js bin didn't work for ie 7-8. so it's still a ?

I read online w3c has some different idea, on how the buttons should be
interpered
left0, middle1, right2 vs ie 0, 1, 4, 2 that will allow for bitmask the
values thus supporting multi press checking.
maybe the easiest way would be to use jQuery's version, that harmonizes
the values.
I know another dependency sucks, and there's always the copy pasta option :)
OR alternatively
place onmousedown and onmouseup events on body and globally set the
state, removing the inconsistent button check altogether.
My money would be on the last one atm. If you'd like I could do it, but
sadly no sooner than next week..

sami

btw browserstack.com rocks! If I only had more than 12min on my trial
left :/

On 30.10.2015 11:32, Philipp Wambach wrote:

I made a small test jsbin
https://jsbin.com/wuyoqadeyu/edit?js,console,output and the result
is as you said: buttons is undefined in Safari.
This is the behaviour on OSX when the left mouse button is pressed
during mousemove:

Chrome Firefox Safari
e.button 0 0 0
e.buttons 1 1 undefined
e.which 1 1 1

and when the button is not pressed (note that on firefox which is
still 1):

Chrome Firefox Safari
e.button 0 0 0
e.buttons 0 0 undefined
e.which 0 1 0

so as you said we have to check if e.which is undefined and use
e.buttons instead
var buttonPressed = (e.which === undefined) ? e.which : e.buttons

|if(e.buttons === undefined) {
//Safari -> use e.which
} else {
//others -> use e.buttons
}

could you check it on Linux? I think IE could be another problem...
|

—
Reply to this email directly or view it on GitHub
#20 (comment).

[w3sami]

Hi Phillip!

Do you want me to code the onmousedown/up stuff for you, or did you
manage it on your own already?

sami

On 10/30/2015 11:32 AM, Philipp Wambach wrote:

I made a small test jsbin
https://jsbin.com/wuyoqadeyu/edit?js,console,output and the result
is as you said: buttons is undefined in Safari.
This is the behaviour on OSX when the left mouse button is pressed
during mousemove:

Chrome Firefox Safari
e.button 0 0 0
e.buttons 1 1 undefined
e.which 1 1 1

and when the button is not pressed (note that on firefox which is
still 1):

Chrome Firefox Safari
e.button 0 0 0
e.buttons 0 0 undefined
e.which 0 1 0

so as you said we have to check if e.which is undefined and use
e.buttons instead
var buttonPressed = (e.which === undefined) ? e.which : e.buttons

|if(e.buttons === undefined) { //Safari -> use e.which } else {
//others -> use e.buttons } could you check it on Linux? I think IE
could be another problem... |

—
Reply to this email directly or view it on GitHub
#20 (comment).

Parhain terveisin,

Sami Lehtilä
Tekninen asiantuntija

sami.lehtila@w3.fi mailto:sami.lehtila@w3.fi, p. 040 595 2898
www.w3.fi http://www.w3.fi
https://www.facebook.com/W3GroupFinland

W3 Group Finland Oy - Käyttökelpoisempia ratkaisuja webiin ja ihmisten
arkeen
Puhelinkeskus: 0207 430 780, Sähköposti: info@w3.fi mailto:info@w3.fi

Kerava: Klondyke-talo Kumitehtaankatu 5, FI-04260 Kerava, Finland
Helsinki: Kuortaneenkatu 5, FI-00520 Helsinki, Finland

[w3sami]

here it is. sorry for the indent, you seem to be using tabs..

[pwambach]

hey, sorry i have been quite busy the last days. i will have a closer look at it soon. one thing we have to consider with this approach is to remove the body listener on the scope destroy event
cheers phil

[w3sami]

good catch! added the remove listeners :)

[pwambach]

thx for your help! i merged your changes and also converted all tabs to spaces :)

[w3sami]

great! glad to be of help :) and thank you for the lib, we're using it to save signatures. the last thing for it to work straight from your repo, is the exposing the canvas object via options. I made the another pull with that change. I was not sure about the tmpcanvas, and atm not actually using it, so I think it could very well be not needed at all. Also, if you have a better idea on how to expose the canvas, by all means. Passing it to the two way config was the easiest and simplest I could think of.

from there I can get the data for saving by just options.canvas.toDataURL()

I might be cleaner though, to build some dedicated stuff for it to avoid confusion?! Just as long there's a way to access it, it's all good :D