Password reveal button not always placed correctly

[simu]

After removing the dependency on jquery for the whole extension, my custom password reveal button is not always positioned correctly in relation to the password field.

Goals

• Improved placement algorithm for the reveal button

[lord2800]

I've encountered this a lot. It would be nice if this could be disabled via a setting as well--I generally don't use that button in favor of the url bar toggle.

[simu]

I haven't had much time to work on the extension recently, I'll look into adding an option to disable the reveal button, as fixing the positioning is rather harder than adding another option.

[lord2800]

I can maybe poke at it if you give me a bit of guidance on where to look.

[simu]

The button is implemented by hand using some javascript in

passwordhasherplus/content-script.js

Lines 57 to 85 in b9c7f7c

	function createMaskButton(field) {
	if (debug) console.log("creating mask button for field " + field.id);
	/* create unmask button */
	var maskbutton = document.createElement('div');
	maskbutton.classList.add('passhashbutton');
	/* position at the bottom right corner of password field */
	var fs = getStyles(field, [ 'display', 'width', 'height',
	'padding-top', 'padding-bottom', 'padding-left', 'padding-right',
	'margin-top', 'margin-bottom', 'margin-left',
	'border-bottom-right-radius', 'border-top-width',
	'border-bottom-width', 'border-left-width', 'border-right-width']);
	if (fs.display == 'none') {
	// shortcircuit when field is display:none
	return maskbutton;
	}
	field.parentNode.insertBefore(maskbutton, field);
	// We get the field dimensions from the field's computed styles
	var margintop = fs.height + fs['border-top-width'] +
	fs['border-bottom-width'] + fs['padding-top'] + fs['padding-bottom'] +
	fs['margin-top'];
	maskbutton.style['margin-top'] = `${margintop}px`;
	if (debug) console.log("maskbutton.margin-top: " + maskbutton.style['margin-top']);
	var marginleft = fs.width - fs['border-bottom-right-radius'] - 30 +
	fs['border-left-width'] + fs['padding-left'] + fs['margin-left'] +
	fs['padding-right'];
	maskbutton.style['margin-left'] = `${marginleft}px`;
	if (debug) console.log("maskbutton.margin-left: " + maskbutton.style['margin-left']);
	return maskbutton;
	}

A new option can be implemented in options.html/options.js and used in content-script.js to not create the button by simply not calling createMaskButton() in bind().

[lord2800]

I created #23 to have an option to disable the button and I'm working on fixing the styles to just anchor it to the bottom right corner. I just need to finish testing my changes, then I'll have a PR for that too.

[lord2800]

I created #25 to fix the button placement as well as a few other minor things. If #23 lands first, I'll have to rebase #25 to handle the new place to call requestAnimationFrame, otherwise I'll have to rebase #23 to do it. Either way, let me know and I'll get the other fixed up.

[simu]

@lord2800 thanks for the contributions, I'll try to have a look ASAP

[simu]

@lord2800 Sorry for the long delay, work was super busy. I've merged #23. #25 LGTM as well under the assumption that the new call site to initAllElements is properly updated.

[lord2800]

On it!

[simu]

While #25 and #28 improve the positioning on some pages, others still struggle, e.g. gitlab.com:

I'm going to leave this issue open for now, in case someone (or myself when I'm bored enough) can try to come up with a better way. I think what's missing is that the content-script.js isn't really able to react correctly when already existing elements are moved around. Maybe there's a way to subscribe on position updates for individual elements and react to those, but that's an idea for another day.

[lord2800]

I'll see what I can do! Knowing that gitlab has an issue still makes it easier for me to test out different solutions.

[simu]

Other pages which I've found where the button is still placed weird are:

• Google
  

[lord2800]

Interesting. I explicitly tested there too. :(

[simu]

maybe my changes to make it work on my default test (https://store.steampowered.com/login/?redir=&redir_ssl=1) (cf. #28) broke it on Google again, but that just shows that it's really tricky to get this right in all cases :(