Skip to content
This repository has been archived by the owner on May 27, 2019. It is now read-only.

WIP: Bug/ff input auto select #158

Merged
merged 5 commits into from
Oct 8, 2017
Merged

WIP: Bug/ff input auto select #158

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5bdd62d
FF input auto select fix: initial prototype
turboMaCk Sep 27, 2017
014aeab
fix code style
turboMaCk Sep 27, 2017
5bd17ac
Merge branch 'master' into bug/ff_input_auto_select
turboMaCk Sep 28, 2017
710a8d0
fix condition
turboMaCk Oct 8, 2017
ee94ee4
Merge branch 'bug/ff_input_auto_select' of github.com:turboMaCk/brows…
turboMaCk Oct 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions chrome/script.browserify.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ var searching = false;
var logins = null;
var domain, urlDuringSearch;

m.mount(document.getElementById("mount"), { view: view });
m.mount(document.getElementById("mount"), { view: view, oncreate: oncreate });

chrome.browserAction.setIcon({ path: "icon-lock.svg" });
chrome.tabs.onActivated.addListener(init);
Expand Down Expand Up @@ -166,15 +166,29 @@ function keyHandler(e) {
}

function switchFocus(firstSelector, nextNodeAttr) {
var inputId = "search-field";
var newActive =
document.activeElement.id === inputId
var searchField = document.getElementById('search-field');
var newActive = document.activeElement === searchField
? document.querySelector(firstSelector)
: document.activeElement[nextNodeAttr];

if (newActive) {
newActive.focus();
} else {
document.getElementById(inputId).focus();
searchField.focus();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a tiny change I forgot to push to the previous PR it seems. I can remove it from this one if you want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need, keep it here 👍

}
}

// The oncreate(vnode) hook is called after a DOM element is created and attached to the document.
// see https://mithril.js.org/lifecycle-methods.html#oncreate for mor informations
function oncreate() {
var searchField = document.getElementById('search-field');

// FireFox probably prevents `focus()` calls for some time
// after extension is rendered.
searchField.focus();
window.setTimeout(function() {
if (searchField) {
searchField.focus();
}
}, 100);
Copy link
Contributor Author

@turboMaCk turboMaCk Sep 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is that ugly hack with timeout:(

EDIT: I've tried to set it to 0 just to run it once call stack is empty - it didn't work though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to wrap it in requestAnimationFrame instead of the setTimeout, I'm curious if that's what FF wants.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also move the call to getElementById inside the requestAnimationFrame, just to make sure we aren't searching too early :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just found some time to try it and it doesn't work which makes sense to me. if window.setTimeout(fc, 0) doesn't work I wouldn't expect requestAnimationFrame to work either. With 0 timeout browser evaluate function right after call-stack is empty. With the request, animation frame code is evaluated even before redraw.

I also tried this sort of "recursive" approach:

function oncreate() {
  var searchField = document.getElementById('search-field');
  searchField.focus();

  if (document.activeElement !== searchField) {
    window.requestAnimationFrame(oncreate);
  }
}

which doesn't work either.

One thing I'm still not sure about is what makes timeout 100 works. Is it guaranteed to always work (since there is some timeout before focus() calls are accepted) or is some race in async code? I don't really know and frankly, I don't know how to test it either. So I did it just by trying. and it seems that with 40ms timeout it sometimes works and sometimes doesn't. Which seems to be answer - timeout might be simply buggy.

Next thing I did was to remove whole setTimeout hack together with autofocus: "on" and test that in chrome. And it worked correctly.

To me, it seems that same thing that prevents us to simply use autofocus in FF is the thing that blocks focus() call. Maybe we can try different DOM events but I'm skeptical.

Another solution that came to my mind was to rewrite that function so it will perform check every 10ms and if it's not successful it will set itself to run in next 10ms until it is. Anyway, I did quick and it seems that document.activeElement returns input even though it's not really selected so there is no (easy) way to perform that test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for doing the experiments! Then let's keep it simple and not over-engineer stuff, it's a bit hacky - yes, but it works and it's quite simple to understand, especially given that you left an explanatory comment. I'll merge now. Thanks 👍

}