Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ema/prevent focus extensions #270

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
59 changes: 47 additions & 12 deletions dist/autofill-debug.js

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions dist/autofill.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 47 additions & 12 deletions dist/autofill.js

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions integration-test/helpers/pages.js
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,14 @@ export function emailAutofillPage (page, server) {
if (!box) throw new Error('unreachable')
await input.click({position: {x: box.width - (box.height / 2), y: box.height / 2}})
},
async assertInputHasFocus () {
const input = page.locator(selectors.identity)
await expect(input).toBeFocused()
},
async assertInputNotFocused () {
const input = page.locator(selectors.identity)
await expect(input).not.toBeFocused()
},
async assertEmailValue (emailAddress) {
const email = page.locator(selectors.identity)
await expect(email).toHaveValue(emailAddress)
Expand Down
2 changes: 2 additions & 0 deletions integration-test/tests/incontext-signup.extension.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ test.describe('chrome extension', () => {
// Confirm tooltip hidden after clicking the input
await emailPage.clickIntoInput()
await incontextSignup.assertIsHidden()
await emailPage.assertInputHasFocus()

// Confirm tooltip shows after clicking the Dax icon
await emailPage.clickDirectlyOnDax()
await emailPage.assertInputNotFocused()
await incontextSignup.assertIsShowing()
await incontextSignup.getEmailProtection()

Expand Down
1 change: 1 addition & 0 deletions src/DeviceInterface.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function createDevice () {
}
return new AppleDeviceInterface(globalConfig, deviceApi, settings)
}
globalConfig.isExtension = true
Copy link
Member Author

Choose a reason for hiding this comment

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

I ended up setting it here. All other similar properties are in the globalConfig so it makes sense, but extensions don't inject strings on the fly so we can do it like this here. Any egregious downside?

FYI, using instanceof ExtensionInterface couldn't be used because imports were not failing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd say for now this is fine given how we're determining that we're in the extension - as you can imagine there are other ways to be more deterministic, but given what we have currently I don't think this is an issue at all :)

return new ExtensionInterface(globalConfig, deviceApi, settings)
}

Expand Down
19 changes: 15 additions & 4 deletions src/Form/Form.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
isEventWithinDax,
isLikelyASubmitButton,
isVisible, buttonMatchesFormType,
safeExecute, getText
safeExecute, getText, wasAutofilledByChrome
} from '../autofill-utils.js'

import {getInputSubtype, getInputMainType, createMatching, safeRegex} from './matching.js'
Expand Down Expand Up @@ -196,6 +196,7 @@ class Form {

removeInputHighlight (input) {
removeInlineStyles(input, getIconStylesAutofilled(input, this))
removeInlineStyles(input, {'cursor': 'pointer'})
input.classList.remove('ddg-autofilled')
this.addAutofillStyles(input)
}
Expand Down Expand Up @@ -374,12 +375,17 @@ class Form {
if (hasIcon) {
this.addAutofillStyles(input)
this.addListener(input, 'mousemove', (e) => {
if (wasAutofilledByChrome(input)) return
shakyShane marked this conversation as resolved.
Show resolved Hide resolved

if (isEventWithinDax(e, e.target)) {
e.target.style.setProperty('cursor', 'pointer', 'important')
addInlineStyles(e.target, {'cursor': 'pointer'})
shakyShane marked this conversation as resolved.
Show resolved Hide resolved
} else {
e.target.style.removeProperty('cursor')
removeInlineStyles(e.target, {'cursor': 'pointer'})
}
})
this.addListener(input, 'mouseleave', (e) => {
removeInlineStyles(e.target, {'cursor': 'pointer'})
})
shakyShane marked this conversation as resolved.
Show resolved Hide resolved
}

function getMainClickCoords (e) {
Expand Down Expand Up @@ -415,6 +421,8 @@ class Form {
const input = e.target
let click = null

if (wasAutofilledByChrome(input)) return

if (!canBeInteractedWith(input)) return

// Checks for pointerdown event
Expand All @@ -428,14 +436,17 @@ class Form {
}

if (this.shouldOpenTooltip(e, input)) {
// On mobile and extensions we don't trigger the focus event to avoid
// keyboard flashing and conflicts with browsers' own tooltips
if (
this.device.globalConfig.isMobileApp &&
(this.device.globalConfig.isMobileApp || this.device.globalConfig.isExtension) &&
shakyShane marked this conversation as resolved.
Show resolved Hide resolved
// Avoid the icon capturing clicks on small fields making it impossible to focus
input.offsetWidth > 50 &&
isEventWithinDax(e, input)
) {
e.preventDefault()
e.stopImmediatePropagation()
input.blur()
shakyShane marked this conversation as resolved.
Show resolved Hide resolved
}

this.touched.add(input)
Expand Down
6 changes: 3 additions & 3 deletions src/Form/inputTypeConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,21 @@ const getIdentitiesIcon = (input, {device}) => {
if (!canBeInteractedWith(input)) return ''

// In Firefox web_accessible_resources could leak a unique user identifier, so we avoid it here
const { isDDGApp, isFirefox } = device.globalConfig
const { isDDGApp, isFirefox, isExtension } = device.globalConfig
const subtype = getInputSubtype(input)

if (subtype === 'emailAddress' && device.inContextSignup?.isAvailable()) {
if (isDDGApp || isFirefox) {
return daxGrayscaleBase64
} else if (typeof window.chrome?.runtime !== 'undefined') {
} else if (isExtension) {
return chrome.runtime.getURL('img/logo-small-grayscale.svg')
}
}

if (subtype === 'emailAddress' && device.isDeviceSignedIn()) {
if (isDDGApp || isFirefox) {
return daxBase64
} else if (typeof window.chrome?.runtime !== 'undefined') {
} else if (device.globalConfig.isExtension) {
return chrome.runtime.getURL('img/logo-small.svg')
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/UI/styles/autofill-tooltip-styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ hr:first-child {

/* Email Protection signup notice */
:not(.top-autofill) .tooltip--email-signup {
text-align: left;
color: #222222;
padding: 16px 20px;
width: 380px;
Expand Down Expand Up @@ -291,6 +292,7 @@ hr:first-child {
border: 0;
cursor: pointer;
display: inline-block;
font-family: inherit;
shakyShane marked this conversation as resolved.
Show resolved Hide resolved
font-style: normal;
font-weight: bold;
padding: 8px 12px;
Expand Down
17 changes: 16 additions & 1 deletion src/autofill-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,20 @@ function isValidTLD (hostname = window.location.hostname) {
return tldrs.test(hostname)
}

/**
* Chrome's UA adds styles using this selector when using the built-in autofill
* @param {HTMLInputElement} input
* @returns {boolean}
*/
const wasAutofilledByChrome = (input) => {
shakyShane marked this conversation as resolved.
Show resolved Hide resolved
try {
// Other browsers throw because the selector is invalid
return input.matches('input:-internal-autofill-selected')
} catch (e) {
return false
}
}

export {
notifyWebApp,
sendAndWaitForAnswer,
Expand All @@ -399,5 +413,6 @@ export {
buttonMatchesFormType,
getText,
isLocalNetwork,
isValidTLD
isValidTLD,
wasAutofilledByChrome
}
2 changes: 2 additions & 0 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,15 @@ function createGlobalConfig (overrides) {
const isMobileApp = ['ios', 'android'].includes(userPreferences?.platform.name) || isAndroid
const isFirefox = navigator.userAgent.includes('Firefox')
const isDDGDomain = Boolean(window.location.href.match(DDG_DOMAIN_REGEX))
const isExtension = false

const config = {
isApp,
isDDGApp,
isAndroid,
isFirefox,
isMobileApp,
isExtension,
isTopFrame,
isWindows,
secret,
Expand Down
1 change: 1 addition & 0 deletions src/device-interface.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ interface GlobalConfig {
isAndroid: boolean;
isFirefox: boolean;
isMobileApp: boolean;
isExtension: boolean;
isWindows: boolean;
isTopFrame: boolean;
secret: string;
Expand Down
59 changes: 47 additions & 12 deletions swift-package/Resources/assets/autofill-debug.js

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions swift-package/Resources/assets/autofill.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 47 additions & 12 deletions swift-package/Resources/assets/autofill.js

Large diffs are not rendered by default.