Commit 6d825968 authored by Yiming Zhou's avatar Yiming Zhou Committed by Commit Bot

Various bug fixes for the Action Recorder Extension.

1. Skip HTML attributes created by Chrome in the xpath builder.
2. Made allowance for the edge case when a page hides the input element that triggers autofill.
3. Remove an unnecessary message exchange between the tab frame content script and the extension background script.
The background script has information on each tab frame.
When the background script sends a message to the content script to start recording, the content script
sends another message back to the background script to obtain the frame information. After this change,
the background script will send the frame information directly in the start recording message.

Bug: 855284
Change-Id: Ifd696f0e4617392ef989e5af0b5d38acd259f467
Reviewed-on: https://chromium-review.googlesource.com/1178306
Commit-Queue: Yiming Zhou <uwyiming@google.com>
Reviewed-by: default avatarJared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#583905}
parent ed1f45d0
...@@ -312,7 +312,7 @@ ...@@ -312,7 +312,7 @@
frameId: targetFrame.parentFrameId frameId: targetFrame.parentFrameId
}) })
.then((frameName) => { .then((frameName) => {
if (frameName !== '') { if (frameName !== '' && frameName !== undefined) {
context.browserTest = { name: frameName }; context.browserTest = { name: frameName };
resolve(context); resolve(context);
} else { } else {
...@@ -699,19 +699,6 @@ ...@@ -699,19 +699,6 @@
if (!request) return false; if (!request) return false;
switch (request.type) { switch (request.type) {
// Tab commands. // Tab commands.
// Query for a frame's frame id and parent frame id.
case RecorderMsgEnum.GET_FRAME_CONTEXT:
getIframeContext(sender.tab.id, sender.frameId, request.location)
.then((context) => {
sendResponse(context);
})
.catch((error) => {
console.error(
`Unable to query for context on tab ${sender.tab.id}, ` +
`frame ${sender.frameId}!\r\n`,
error);
});
return true;
case RecorderMsgEnum.SAVE: case RecorderMsgEnum.SAVE:
downloadRecipe() downloadRecipe()
.then(() => sendResponse(true)); .then(() => sendResponse(true));
......
...@@ -54,11 +54,13 @@ ...@@ -54,11 +54,13 @@
// Disabled. Class list is simply too mutable, especially in the // Disabled. Class list is simply too mutable, especially in the
// case where an element changes class on hover or on focus. // case where an element changes class on hover or on focus.
continue; continue;
} else if (attr === 'title') { } else if (attr === 'autofill-prediction' ||
// 'title' is an attribute inserted by Chrome Autofill to predict attr === 'field_signature' ||
// how the field should be filled. Since this attribute is inserted attr === 'pm_parser_annotation' ||
// by Chrome, it may change from build to build. Skip this attr === 'title') {
// attribute. // These attributes are inserted by Chrome.
// Since Chrome sets these attributes, these attributes may change
// from build to build. Skip these attributes.
continue; continue;
} else { } else {
attributes.push(`@${attr}`); attributes.push(`@${attr}`);
...@@ -174,8 +176,8 @@ ...@@ -174,8 +176,8 @@
return build(target); return build(target);
}; };
let autofillTriggerElementSelector = null; let autofillTriggerElementInfo = null;
let lastTypingEventTargetValue = null; let lastTypingEventInfo = null;
let frameContext; let frameContext;
let mutationObserver = null; let mutationObserver = null;
let started = false; let started = false;
...@@ -197,6 +199,18 @@ ...@@ -197,6 +199,18 @@
return element.getAttribute('type') === 'password'; return element.getAttribute('type') === 'password';
} }
function isChromeRecognizedPasswordField(element) {
const passwordManagerParserAnnotation =
element.getAttribute('pm_parser_annotation');
return passwordManagerParserAnnotation === 'password_element' ||
passwordManagerParserAnnotation === 'new_password_element' ||
passwordManagerParserAnnotation === 'confirmation_password_element';
}
function isChromeRecognizedUserNameField(element) {
return element.getAttribute('pm_parser_annotation') === 'username_element';
}
function canTriggerAutofill(element) { function canTriggerAutofill(element) {
return (element.localName === 'input' && return (element.localName === 'input' &&
['checkbox', 'radio', 'button', 'submit', 'hidden', 'reset'] ['checkbox', 'radio', 'button', 'submit', 'hidden', 'reset']
...@@ -288,7 +302,9 @@ ...@@ -288,7 +302,9 @@
} }
addActionToRecipe(action); addActionToRecipe(action);
} }
} else if (lastTypingEventTargetValue === event.target.value) { } else if (lastTypingEventInfo &&
lastTypingEventInfo.target === event.target &&
lastTypingEventInfo.value === event.target.value) {
console.log(`Typing detected on: ${selector}`); console.log(`Typing detected on: ${selector}`);
// Distinguish between typing inside password input fields and // Distinguish between typing inside password input fields and
...@@ -317,11 +333,16 @@ ...@@ -317,11 +333,16 @@
} else { } else {
// If the user has previously clicked on a field that can trigger // If the user has previously clicked on a field that can trigger
// autofill, add a trigger autofill action. // autofill, add a trigger autofill action.
if (autofillTriggerElementSelector !== null) { if (autofillTriggerElementInfo !== null) {
console.log(`Triggered autofill on ${autofillTriggerElementSelector}`); console.log(`Triggered autofill on ${autofillTriggerElementInfo.selector}`);
action.type = 'autofill'; let autofillAction = {
addActionToRecipe(action); selector: autofillTriggerElementInfo.selector,
autofillTriggerElementSelector = null; context: frameContext,
visibility: autofillTriggerElementInfo.visibility
};
autofillAction.type = 'autofill';
addActionToRecipe(autofillAction);
autofillTriggerElementInfo = null;
} }
action.type = 'validateField'; action.type = 'validateField';
action.expectedValue = event.target.value; action.expectedValue = event.target.value;
...@@ -367,7 +388,10 @@ ...@@ -367,7 +388,10 @@
// the element selector path, as the user could have clicked // the element selector path, as the user could have clicked
// this element to trigger autofill. // this element to trigger autofill.
if (isAutofillableElement(element) && canTriggerAutofill(element)) { if (isAutofillableElement(element) && canTriggerAutofill(element)) {
autofillTriggerElementSelector = selector; autofillTriggerElementInfo = {
selector: selector,
visibility: elementReadyState
}
} }
} else { } else {
addActionToRecipe({ addActionToRecipe({
...@@ -376,7 +400,7 @@ ...@@ -376,7 +400,7 @@
context: frameContext, context: frameContext,
type: 'click' type: 'click'
}); });
autofillTriggerElementSelector = null; autofillTriggerElementInfo = null;
} }
} else if (event.button === Buttons.RIGHT_BUTTON) { } else if (event.button === Buttons.RIGHT_BUTTON) {
const element = event.target; const element = event.target;
...@@ -407,19 +431,16 @@ ...@@ -407,19 +431,16 @@
} }
if (isEditableInputElement(event.target)) { if (isEditableInputElement(event.target)) {
lastTypingEventTargetValue = event.target.value; lastTypingEventInfo = {
target: event.target,
value: event.target.value
}
} else { } else {
lastTypingEventTargetValue = null; lastTypingEventInfo = null;
} }
} }
function startRecording() { function startRecording(context) {
const promise =
// First, obtain the current frame's context.
sendRuntimeMessageToBackgroundScript({
type: RecorderMsgEnum.GET_FRAME_CONTEXT,
location: location})
.then((context) => {
frameContext = context; frameContext = context;
// Register on change listeners on all the input elements. // Register on change listeners on all the input elements.
registerOnInputChangeActionListener(document); registerOnInputChangeActionListener(document);
...@@ -457,8 +478,6 @@ ...@@ -457,8 +478,6 @@
mutationObserver.observe(document, {childList: true, subtree: true}); mutationObserver.observe(document, {childList: true, subtree: true});
started = true; started = true;
return Promise.resolve(); return Promise.resolve();
});
return promise;
} }
function stopRecording() { function stopRecording() {
...@@ -476,8 +495,7 @@ ...@@ -476,8 +495,7 @@
const iframes = document.querySelectorAll('iframe'); const iframes = document.querySelectorAll('iframe');
// Find the target iframe. // Find the target iframe.
for (let index = 0; index < iframes.length; index++) { for (let index = 0; index < iframes.length; index++) {
const url = new URL(iframes[index].src, const url = new URL(iframes[index].src, location.origin);
`${location.protocol}//${location.host}`);
// Try to identify the iframe using the entire URL. // Try to identify the iframe using the entire URL.
if (frameLocation.href === url.href) { if (frameLocation.href === url.href) {
iframe = iframes[index]; iframe = iframes[index];
...@@ -490,8 +508,7 @@ ...@@ -490,8 +508,7 @@
// To handle the scenario described above, this code optionally ignores // To handle the scenario described above, this code optionally ignores
// the iframe url's hash. // the iframe url's hash.
if (iframes === null && if (iframes === null &&
frameLocation.protocol === url.protocol && frameLocation.origin === url.origin &&
frameLocation.host === url.host &&
frameLocation.pathname === url.pathname && frameLocation.pathname === url.pathname &&
frameLocation.search === url.search) { frameLocation.search === url.search) {
iframe = iframes[index]; iframe = iframes[index];
...@@ -514,7 +531,7 @@ ...@@ -514,7 +531,7 @@
if (!request) return; if (!request) return;
switch (request.type) { switch (request.type) {
case RecorderMsgEnum.START: case RecorderMsgEnum.START:
startRecording() startRecording(request.frameContext)
.then(() => sendResponse(true)) .then(() => sendResponse(true))
.catch((error) => { .catch((error) => {
sendResponse(false); sendResponse(false);
...@@ -528,6 +545,7 @@ ...@@ -528,6 +545,7 @@
sendResponse(true); sendResponse(true);
break; break;
case RecorderMsgEnum.GET_IFRAME_NAME: case RecorderMsgEnum.GET_IFRAME_NAME:
console.log(`Cross: ${request.url}`);
queryIframeName(request.url) queryIframeName(request.url)
.then((context) => { .then((context) => {
sendResponse(context); sendResponse(context);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment