Remove initial handlers #195
Conversation
|
These are used when switching tabs. Unfortunately it's hard to test with Selenium so the tests are turned off. I think we need to keep these in because it helps us start the polyfill by ensuring the user is in keyboard mode and swapping them out of keyboard mode if we detect a gesture. Are the listeners adversely affecting your project? I think the solution is a little wonky but wasn't able to come up with a better one at the time :D |
I tested it locally and didn't have any issues. Could you provide concrete steps that would fail without them? Just by looking at them they don't seem to affect
https://xem.github.io/terser-online/ -Output size: 2540b
+Output size: 1685bhttp://www.txtwizard.net/compression -Execution time: 51335 us Compression ratio: 248 % Original size: 1686 bytes Result size: 680 bytes
+Execution time: 60727 us Compression ratio: 318 % Original size: 2541 bytes Result size: 800 bytesAnd it's just quite a bit of code. I don't want to put in code that I don't understand or that isn't tested. |
|
I think the only other issue was related to tabbing into the page from the URL bar but maybe that's satisfied by setting hadKeyboardEvent = true when the page first loads. Sorry I don't have much time to test, Google I/O is a week away and we're all heads down over here 😅 Happy to consider removing it if y'all are noticing it running ok in your fork and once i've had a chance to resurface from I/O craziness. |
Will do. We didn't consider this case before so it's fine for now to continue doing so. We can improve this later. Godspeed for I/O and feel free to revisit this once you have enough time. |
|
I just tested the pull request. Under MacOS 10.14.5 and Safari 12.1.1, the polyfill no longer works as desired, but the focus is displayed again in certain situations (e.g. hovering elements). |
@joshuaruesweg Could you elaborate in what circumstances which expected behavior did not occur? |
|
Hello, sorry, actually I should have tested the whole thing in more detail in my commentary. It seems to work in Safari without any problems. I've tested it more extensively now, sorry for the confusion, I don't know what put a spoke in my wheel at the first test. |
This might seem very naive but no test broke after removing them.
I could only find a single note about a possible safari quirk (safari is currently not tested as far as I can tell) but could not replicate any issue in browserstack (win10+safari 5.1 or os x snow leopard + safari4).
So this is more of a question for what browser + os combination this is required for. Since this is a lot of code I would probably drop it in a fork if this only occurs for odd os + browser combinations.