From d4001a4a982d05af102cfa9a5a6427d19f91198e Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Wed, 6 Dec 2017 22:37:00 -0500 Subject: [PATCH 01/18] Add support for hinting elements from within same-origin frames --- qutebrowser/javascript/webelem.js | 46 +++++++++++++++++++++++----- tests/end2end/features/hints.feature | 2 -- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index b5fd936b2..f8aed2983 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -40,7 +40,7 @@ window._qutebrowser.webelem = (function() { const funcs = {}; const elements = []; - function serialize_elem(elem) { + function serialize_elem(elem, frame = null) { if (!elem) { return null; } @@ -55,7 +55,7 @@ window._qutebrowser.webelem = (function() { try { caret_position = elem.selectionStart; } catch (err) { - if (err instanceof DOMException && + if (err.constructor.name === "DOMException" && err.name === "InvalidStateError") { // nothing to do, caret_position is already null } else { @@ -102,15 +102,25 @@ window._qutebrowser.webelem = (function() { out.attributes = attributes; const client_rects = elem.getClientRects(); + + // Get location of frame and add it to element + // TODO How to generate a 0 object without this + let frame_offset_rect = null; + if (frame === null) { + frame_offset_rect = document.head.getBoundingClientRect(); + } else { + frame_offset_rect = frame.getBoundingClientRect(); + } + for (let k = 0; k < client_rects.length; ++k) { const rect = client_rects[k]; out.rects.push({ - "top": rect.top, - "right": rect.right, - "bottom": rect.bottom, - "left": rect.left, - "height": rect.height, - "width": rect.width, + "top": rect.top + frame_offset_rect.top, + "right": rect.right + frame_offset_rect.right, + "bottom": rect.bottom + frame_offset_rect.bottom, + "left": rect.left + frame_offset_rect.left, + "height": rect.height + frame_offset_rect.height, + "width": rect.width + frame_offset_rect.width, }); } @@ -163,6 +173,7 @@ window._qutebrowser.webelem = (function() { funcs.find_css = (selector, only_visible) => { const elems = document.querySelectorAll(selector); + const subelem_frames = window.frames; const out = []; for (let i = 0; i < elems.length; ++i) { @@ -171,6 +182,25 @@ window._qutebrowser.webelem = (function() { } } + // Recurse into frames and add them + for (let i = 0; i < subelem_frames.length; i++) { + try { + subelem_frames[i].document; + } catch (err) { + // If we have a cross-origin frame, skip it. + continue; + } + + const subelems = subelem_frames[i].document. + querySelectorAll(selector); + const frame = subelem_frames[i].frameElement; + for (let elem_num = 0; elem_num < subelems.length; ++elem_num) { + if (!only_visible || is_visible(subelems[elem_num])) { + out.push(serialize_elem(subelems[elem_num], frame)); + } + } + } + return out; }; diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index 0151a14ad..0f7868e31 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -204,8 +204,6 @@ Feature: Using hints Then the javascript message "contents: existingnew" should be logged ### iframes - - @qtwebengine_todo: Hinting in iframes is not implemented yet Scenario: Using :follow-hint inside an iframe When I open data/hints/iframe.html And I hint with args "links normal" and follow a From c737d7ab2237f9fb768367fd55090e870ab1c22b Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Thu, 7 Dec 2017 12:47:51 -0500 Subject: [PATCH 02/18] Fix various js problems with frame support --- qutebrowser/javascript/webelem.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index f8aed2983..51005f882 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -55,7 +55,9 @@ window._qutebrowser.webelem = (function() { try { caret_position = elem.selectionStart; } catch (err) { - if (err.constructor.name === "DOMException" && + if ((err instanceof DOMException || + (frame !== null && + err instanceof frame.DOMException)) && err.name === "InvalidStateError") { // nothing to do, caret_position is already null } else { @@ -107,9 +109,17 @@ window._qutebrowser.webelem = (function() { // TODO How to generate a 0 object without this let frame_offset_rect = null; if (frame === null) { - frame_offset_rect = document.head.getBoundingClientRect(); + // Dummy object with zero offset + frame_offset_rect = { + "top": 0, + "right": 0, + "bottom": 0, + "left": 0, + "height": 0, + "width": 0, + }; } else { - frame_offset_rect = frame.getBoundingClientRect(); + frame_offset_rect = frame.frameElement.getBoundingClientRect(); } for (let k = 0; k < client_rects.length; ++k) { @@ -193,7 +203,7 @@ window._qutebrowser.webelem = (function() { const subelems = subelem_frames[i].document. querySelectorAll(selector); - const frame = subelem_frames[i].frameElement; + const frame = subelem_frames[i]; for (let elem_num = 0; elem_num < subelems.length; ++elem_num) { if (!only_visible || is_visible(subelems[elem_num])) { out.push(serialize_elem(subelems[elem_num], frame)); From 052823b74cacb06db9edd6c67be919801aa7640c Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Thu, 7 Dec 2017 13:03:06 -0500 Subject: [PATCH 03/18] Fix broken width and height location in frames --- qutebrowser/javascript/webelem.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 51005f882..eeded152d 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -129,8 +129,8 @@ window._qutebrowser.webelem = (function() { "right": rect.right + frame_offset_rect.right, "bottom": rect.bottom + frame_offset_rect.bottom, "left": rect.left + frame_offset_rect.left, - "height": rect.height + frame_offset_rect.height, - "width": rect.width + frame_offset_rect.width, + "height": rect.height, + "width": rect.width, }); } From 0fc99108bf57ce568d2dd91636779e0b133f2bb4 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Thu, 7 Dec 2017 14:21:44 -0500 Subject: [PATCH 04/18] Implement iframe support for clicking elements --- qutebrowser/javascript/webelem.js | 78 +++++++++++++++++++++++-------- 1 file changed, 58 insertions(+), 20 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index eeded152d..8fb5ae1ea 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -181,6 +181,17 @@ window._qutebrowser.webelem = (function() { return true; } + // Returns true if the iframe is accessible without + // cross domain errors, else false. + function iframe_same_domain(frame) { + try { + frame.document; + return true; + } catch (err) { + return false; + } + } + funcs.find_css = (selector, only_visible) => { const elems = document.querySelectorAll(selector); const subelem_frames = window.frames; @@ -194,19 +205,14 @@ window._qutebrowser.webelem = (function() { // Recurse into frames and add them for (let i = 0; i < subelem_frames.length; i++) { - try { - subelem_frames[i].document; - } catch (err) { - // If we have a cross-origin frame, skip it. - continue; - } - - const subelems = subelem_frames[i].document. - querySelectorAll(selector); - const frame = subelem_frames[i]; - for (let elem_num = 0; elem_num < subelems.length; ++elem_num) { - if (!only_visible || is_visible(subelems[elem_num])) { - out.push(serialize_elem(subelems[elem_num], frame)); + if (iframe_same_domain(subelem_frames[i])) { + const subelems = subelem_frames[i].document. + querySelectorAll(selector); + const frame = subelem_frames[i]; + for (let elem_num = 0; elem_num < subelems.length; ++elem_num) { + if (!only_visible || is_visible(subelems[elem_num])) { + out.push(serialize_elem(subelems[elem_num], frame)); + } } } } @@ -219,6 +225,20 @@ window._qutebrowser.webelem = (function() { return serialize_elem(elem); }; + // Check if elem is an iframe, and if so, run func on it. + // If no iframes match, return null + function run_in_iframes(elem, func) { + for (let i = 0; i < window.frames.length; ++i) { + if (iframe_same_domain(window.frames[i])) { + const frame = window.frames[i]; + if (frame.frameElement === elem) { + return func(frame); + } + } + } + return null; + } + funcs.find_focused = () => { const elem = document.activeElement; @@ -228,17 +248,35 @@ window._qutebrowser.webelem = (function() { return null; } - return serialize_elem(elem); + // Check if we got an iframe, and if so, recurse inside of it + const frame_elem = run_in_iframes(elem, + (frame) => serialize_elem(frame.document.activeElement, frame)); + + if (frame_elem === null) { + return serialize_elem(elem); + } + return frame_elem; }; funcs.find_at_pos = (x, y) => { - // FIXME:qtwebengine - // If the element at the specified point belongs to another document - // (for example, an iframe's subdocument), the subdocument's parent - // element is returned (the iframe itself). - const elem = document.elementFromPoint(x, y); - return serialize_elem(elem); + + + // Check if we got an iframe, and if so, recurse inside of it + const frame_elem = run_in_iframes(elem, + (frame) => { + // Subtract offsets due to being in an iframe + const frame_offset_rect = + frame.frameElement.getBoundingClientRect(); + return serialize_elem(frame.document. + elementFromPoint(x - frame_offset_rect.left, + y - frame_offset_rect.top), frame); + }); + + if (frame_elem === null) { + return serialize_elem(elem); + } + return frame_elem; }; // Function for returning a selection to python (so we can click it) From 825939633a8482beb7dd89f69d9c8585f2906244 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Thu, 7 Dec 2017 14:46:18 -0500 Subject: [PATCH 05/18] Implement follow_selected in frames --- qutebrowser/javascript/webelem.js | 38 ++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 8fb5ae1ea..8ce59d5e6 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -227,7 +227,7 @@ window._qutebrowser.webelem = (function() { // Check if elem is an iframe, and if so, run func on it. // If no iframes match, return null - function run_in_iframes(elem, func) { + function replace_elem_frame(elem, func) { for (let i = 0; i < window.frames.length; ++i) { if (iframe_same_domain(window.frames[i])) { const frame = window.frames[i]; @@ -239,6 +239,20 @@ window._qutebrowser.webelem = (function() { return null; } + // Runs a function in a frame until the result is not null, then return + function run_frames(func) { + for (let i = 0; i < window.frames.length; ++i) { + if (iframe_same_domain(window.frames[i])) { + const frame = window.frames[i]; + const result = func(frame); + if (result) { + return result; + } + } + } + return null; + } + funcs.find_focused = () => { const elem = document.activeElement; @@ -249,7 +263,7 @@ window._qutebrowser.webelem = (function() { } // Check if we got an iframe, and if so, recurse inside of it - const frame_elem = run_in_iframes(elem, + const frame_elem = replace_elem_frame(elem, (frame) => serialize_elem(frame.document.activeElement, frame)); if (frame_elem === null) { @@ -263,7 +277,7 @@ window._qutebrowser.webelem = (function() { // Check if we got an iframe, and if so, recurse inside of it - const frame_elem = run_in_iframes(elem, + const frame_elem = replace_elem_frame(elem, (frame) => { // Subtract offsets due to being in an iframe const frame_offset_rect = @@ -282,10 +296,22 @@ window._qutebrowser.webelem = (function() { // Function for returning a selection to python (so we can click it) funcs.find_selected_link = () => { const elem = window.getSelection().anchorNode; - if (!elem) { - return null; + if (elem) { + return serialize_elem(elem.parentNode); } - return serialize_elem(elem.parentNode); + + const serialized_frame_elem = run_frames((frame) => { + const node = frame.window.getSelection().anchorNode; + if (node) { + return serialize_elem(node.parentNode, frame); + } + return null; + }); + + if (serialized_frame_elem) { + return serialized_frame_elem; + } + return null; }; funcs.set_value = (id, value) => { From 5c5f9928219f1d72942ed7ddd55c7a2e1549e406 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Thu, 7 Dec 2017 14:53:15 -0500 Subject: [PATCH 06/18] Implement find_id inside frames Fixes :click-element --- qutebrowser/javascript/webelem.js | 46 +++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 8ce59d5e6..167634721 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -220,9 +220,39 @@ window._qutebrowser.webelem = (function() { return out; }; + // Runs a function in a frame until the result is not null, then return + function run_frames(func) { + for (let i = 0; i < window.frames.length; ++i) { + if (iframe_same_domain(window.frames[i])) { + const frame = window.frames[i]; + const result = func(frame); + if (result) { + return result; + } + } + } + return null; + } + funcs.find_id = (id) => { const elem = document.getElementById(id); - return serialize_elem(elem); + if (elem) { + return serialize_elem(elem); + } + + const serialized_elem = run_frames((frame) => { + const element = frame.window.document.getElementById(id); + if (element) { + return serialize_elem(element, frame); + } + return null; + }); + + if (serialized_elem) { + return serialized_elem; + } + + return null; }; // Check if elem is an iframe, and if so, run func on it. @@ -239,20 +269,6 @@ window._qutebrowser.webelem = (function() { return null; } - // Runs a function in a frame until the result is not null, then return - function run_frames(func) { - for (let i = 0; i < window.frames.length; ++i) { - if (iframe_same_domain(window.frames[i])) { - const frame = window.frames[i]; - const result = func(frame); - if (result) { - return result; - } - } - } - return null; - } - funcs.find_focused = () => { const elem = document.activeElement; From 2898c416aa974eaed08da3af9d0f84b03c58b8b1 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Fri, 15 Dec 2017 15:16:18 -0500 Subject: [PATCH 07/18] Simplify and clean up frame logic --- qutebrowser/javascript/webelem.js | 34 ++++++++++++------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 167634721..ae2e2ca5c 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -105,8 +105,7 @@ window._qutebrowser.webelem = (function() { const client_rects = elem.getClientRects(); - // Get location of frame and add it to element - // TODO How to generate a 0 object without this + // Get location of frame if it exists and add it to element let frame_offset_rect = null; if (frame === null) { // Dummy object with zero offset @@ -206,9 +205,9 @@ window._qutebrowser.webelem = (function() { // Recurse into frames and add them for (let i = 0; i < subelem_frames.length; i++) { if (iframe_same_domain(subelem_frames[i])) { - const subelems = subelem_frames[i].document. - querySelectorAll(selector); const frame = subelem_frames[i]; + const subelems = frame.document. + querySelectorAll(selector); for (let elem_num = 0; elem_num < subelems.length; ++elem_num) { if (!only_visible || is_visible(subelems[elem_num])) { out.push(serialize_elem(subelems[elem_num], frame)); @@ -223,8 +222,8 @@ window._qutebrowser.webelem = (function() { // Runs a function in a frame until the result is not null, then return function run_frames(func) { for (let i = 0; i < window.frames.length; ++i) { - if (iframe_same_domain(window.frames[i])) { - const frame = window.frames[i]; + const frame = window.frames[i]; + if (iframe_same_domain(frame)) { const result = func(frame); if (result) { return result; @@ -242,10 +241,7 @@ window._qutebrowser.webelem = (function() { const serialized_elem = run_frames((frame) => { const element = frame.window.document.getElementById(id); - if (element) { - return serialize_elem(element, frame); - } - return null; + return serialize_elem(element, frame); }); if (serialized_elem) { @@ -255,12 +251,12 @@ window._qutebrowser.webelem = (function() { return null; }; - // Check if elem is an iframe, and if so, run func on it. + // Check if elem is an iframe, and if so, return the result of func on it. // If no iframes match, return null - function replace_elem_frame(elem, func) { + function call_if_frame(elem, func) { for (let i = 0; i < window.frames.length; ++i) { - if (iframe_same_domain(window.frames[i])) { - const frame = window.frames[i]; + const frame = window.frames[i]; + if (iframe_same_domain(frame)) { if (frame.frameElement === elem) { return func(frame); } @@ -279,7 +275,7 @@ window._qutebrowser.webelem = (function() { } // Check if we got an iframe, and if so, recurse inside of it - const frame_elem = replace_elem_frame(elem, + const frame_elem = call_if_frame(elem, (frame) => serialize_elem(frame.document.activeElement, frame)); if (frame_elem === null) { @@ -293,7 +289,7 @@ window._qutebrowser.webelem = (function() { // Check if we got an iframe, and if so, recurse inside of it - const frame_elem = replace_elem_frame(elem, + const frame_elem = call_if_frame(elem, (frame) => { // Subtract offsets due to being in an iframe const frame_offset_rect = @@ -323,11 +319,7 @@ window._qutebrowser.webelem = (function() { } return null; }); - - if (serialized_frame_elem) { - return serialized_frame_elem; - } - return null; + return serialized_frame_elem; }; funcs.set_value = (id, value) => { From 7f9d4888fd3370628b5038f8591424f8d822daf7 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Fri, 15 Dec 2017 15:42:48 -0500 Subject: [PATCH 08/18] Fix a couple eslint errors Restructure serialize_elem into a bunch of smaller functions --- qutebrowser/javascript/.eslintrc.yaml | 1 + qutebrowser/javascript/webelem.js | 73 ++++++++++++++------------- 2 files changed, 39 insertions(+), 35 deletions(-) diff --git a/qutebrowser/javascript/.eslintrc.yaml b/qutebrowser/javascript/.eslintrc.yaml index b29deaa75..d46fec8ee 100644 --- a/qutebrowser/javascript/.eslintrc.yaml +++ b/qutebrowser/javascript/.eslintrc.yaml @@ -42,3 +42,4 @@ rules: function-paren-newline: "off" multiline-comment-style: "off" no-bitwise: "off" + no-ternary: "off" diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index ae2e2ca5c..12459f148 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -40,6 +40,41 @@ window._qutebrowser.webelem = (function() { const funcs = {}; const elements = []; + function get_frame_offset(frame) { + if (frame === null) { + // Dummy object with zero offset + return { + "top": 0, + "right": 0, + "bottom": 0, + "left": 0, + "height": 0, + "width": 0, + }; + } + return frame.frameElement.getBoundingClientRect(); + } + + function get_caret_position(elem, frame) { + // With older Chromium versions (and QtWebKit), InvalidStateError will + // be thrown if elem doesn't have selectionStart. + // With newer Chromium versions (>= Qt 5.10), we get null. + try { + return elem.selectionStart; + } catch (err) { + if (err instanceof (frame + ? frame.DOMException + : DOMException) && + err.name === "InvalidStateError") { + // nothing to do, caret_position is already null + } else { + // not the droid we're looking for + throw err; + } + } + return null; + } + function serialize_elem(elem, frame = null) { if (!elem) { return null; @@ -48,23 +83,7 @@ window._qutebrowser.webelem = (function() { const id = elements.length; elements[id] = elem; - // With older Chromium versions (and QtWebKit), InvalidStateError will - // be thrown if elem doesn't have selectionStart. - // With newer Chromium versions (>= Qt 5.10), we get null. - let caret_position = null; - try { - caret_position = elem.selectionStart; - } catch (err) { - if ((err instanceof DOMException || - (frame !== null && - err instanceof frame.DOMException)) && - err.name === "InvalidStateError") { - // nothing to do, caret_position is already null - } else { - // not the droid we're looking for - throw err; - } - } + const caret_position = get_caret_position(elem, frame); const out = { "id": id, @@ -73,7 +92,6 @@ window._qutebrowser.webelem = (function() { "rects": [], // Gets filled up later "caret_position": caret_position, }; - // https://github.com/qutebrowser/qutebrowser/issues/2569 if (typeof elem.tagName === "string") { out.tag_name = elem.tagName; @@ -104,22 +122,7 @@ window._qutebrowser.webelem = (function() { out.attributes = attributes; const client_rects = elem.getClientRects(); - - // Get location of frame if it exists and add it to element - let frame_offset_rect = null; - if (frame === null) { - // Dummy object with zero offset - frame_offset_rect = { - "top": 0, - "right": 0, - "bottom": 0, - "left": 0, - "height": 0, - "width": 0, - }; - } else { - frame_offset_rect = frame.frameElement.getBoundingClientRect(); - } + const frame_offset_rect = get_frame_offset(frame); for (let k = 0; k < client_rects.length; ++k) { const rect = client_rects[k]; @@ -184,7 +187,7 @@ window._qutebrowser.webelem = (function() { // cross domain errors, else false. function iframe_same_domain(frame) { try { - frame.document; + frame.document; // eslint-disable-line no-unused-expressions return true; } catch (err) { return false; From 6433096611cd99477f730312282da4711bc8b6e3 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Fri, 15 Dec 2017 21:30:08 -0500 Subject: [PATCH 09/18] Disable max-lines in eslint --- qutebrowser/javascript/.eslintrc.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/qutebrowser/javascript/.eslintrc.yaml b/qutebrowser/javascript/.eslintrc.yaml index d46fec8ee..5382f053e 100644 --- a/qutebrowser/javascript/.eslintrc.yaml +++ b/qutebrowser/javascript/.eslintrc.yaml @@ -43,3 +43,4 @@ rules: multiline-comment-style: "off" no-bitwise: "off" no-ternary: "off" + max-lines: "off" From 344ebed6ad2894b7152b0fc4f71a7c4eda00a0db Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Mon, 18 Dec 2017 11:00:03 -0800 Subject: [PATCH 10/18] Add iframe tests for insert on click and follow-selected --- tests/end2end/data/hints/html/wrapped.html | 1 - .../data/hints/html/wrapped_button.html | 23 +++++++++++++++++++ tests/end2end/data/hints/iframe_button.html | 11 +++++++++ tests/end2end/data/hints/iframe_input.html | 11 +++++++++ tests/end2end/data/iframe_search.html | 11 +++++++++ tests/end2end/features/hints.feature | 14 ++++++++++- tests/end2end/features/search.feature | 19 +++++++++++++++ 7 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 tests/end2end/data/hints/html/wrapped_button.html create mode 100644 tests/end2end/data/hints/iframe_button.html create mode 100644 tests/end2end/data/hints/iframe_input.html create mode 100644 tests/end2end/data/iframe_search.html diff --git a/tests/end2end/data/hints/html/wrapped.html b/tests/end2end/data/hints/html/wrapped.html index 2ebde1a24..2af1a28c1 100644 --- a/tests/end2end/data/hints/html/wrapped.html +++ b/tests/end2end/data/hints/html/wrapped.html @@ -2,7 +2,6 @@ diff --git a/tests/end2end/data/hints/html/wrapped_button.html b/tests/end2end/data/hints/html/wrapped_button.html new file mode 100644 index 000000000..dc96bd1d5 --- /dev/null +++ b/tests/end2end/data/hints/html/wrapped_button.html @@ -0,0 +1,23 @@ + + + + + + + + Link wrapped across multiple lines + + +
+ Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. +
+ + + + diff --git a/tests/end2end/data/hints/iframe_button.html b/tests/end2end/data/hints/iframe_button.html new file mode 100644 index 000000000..cd7bf203c --- /dev/null +++ b/tests/end2end/data/hints/iframe_button.html @@ -0,0 +1,11 @@ + + + + + + Hinting inside an iframe + + + + + diff --git a/tests/end2end/data/hints/iframe_input.html b/tests/end2end/data/hints/iframe_input.html new file mode 100644 index 000000000..143577747 --- /dev/null +++ b/tests/end2end/data/hints/iframe_input.html @@ -0,0 +1,11 @@ + + + + + + Hinting inside an iframe + + + + + diff --git a/tests/end2end/data/iframe_search.html b/tests/end2end/data/iframe_search.html new file mode 100644 index 000000000..a06c0ef3e --- /dev/null +++ b/tests/end2end/data/iframe_search.html @@ -0,0 +1,11 @@ + + + + + + Hinting inside an iframe + + + + + diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index 0f7868e31..f575d0423 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -209,6 +209,19 @@ Feature: Using hints And I hint with args "links normal" and follow a Then "navigation request: url http://localhost:*/data/hello.txt, type NavigationTypeLinkClicked, *" should be logged + Scenario: Using :follow-hint inside an iframe button + When I open data/hints/iframe_button.html + And I hint with args "all normal" and follow s + Then "navigation request: url http://localhost:*/data/hello.txt, type NavigationTypeLinkClicked, *" should be logged + + Scenario: Hinting inputs in an iframe without type + When I open data/hints/iframe_input.html + And I hint with args "inputs" and follow a + And I wait for "Entering mode KeyMode.insert (reason: clicking input)" in the log + And I run :leave-mode + # The actual check is already done above + Then no crash should happen + ### FIXME currenly skipped, see https://github.com/qutebrowser/qutebrowser/issues/1525 @xfail_norun Scenario: Using :follow-hint inside a scrolled iframe @@ -218,7 +231,6 @@ Feature: Using hints And I hint wht args "links normal" and follow a Then "navigation request: url http://localhost:*/data/hello2.txt, type NavigationTypeLinkClicked, *" should be logged - @qtwebengine_skip: Opens in new tab due to Chromium bug Scenario: Opening a link inside a specific iframe When I open data/hints/iframe_target.html And I hint with args "links normal" and follow a diff --git a/tests/end2end/features/search.feature b/tests/end2end/features/search.feature index 3778f963d..483bcc29f 100644 --- a/tests/end2end/features/search.feature +++ b/tests/end2end/features/search.feature @@ -238,3 +238,22 @@ Feature: Searching on a page Then the following tabs should be open: - data/search.html - data/hello.txt (active) + + Scenario: Follow a searched link in an iframe + When I open data/iframe_search.html + And I run :tab-only + And I run :search follow + And I wait for "search found follow" in the log + And I run :follow-selected + Then "navigation request: url http://localhost:*/data/hello.txt, type NavigationTypeLinkClicked, is_main_frame False" should be logged + + Scenario: Follow a tabbed searched link in an iframe + When I open data/iframe_search.html + And I run :tab-only + And I run :search follow + And I wait for "search found follow" in the log + And I run :follow-selected -t + And I wait until data/hello.txt is loaded + Then the following tabs should be open: + - data/iframe_search.html + - data/hello.txt (active) From b87f0b6f65d5ca7d05426ee3397fb724e667834a Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Mon, 18 Dec 2017 17:04:50 -0800 Subject: [PATCH 11/18] Add support for non-link buttons to test_hints --- tests/end2end/test_hints_html.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/end2end/test_hints_html.py b/tests/end2end/test_hints_html.py index abc106505..ff4837020 100644 --- a/tests/end2end/test_hints_html.py +++ b/tests/end2end/test_hints_html.py @@ -106,7 +106,11 @@ def test_hints(test_name, zoom_text_only, zoom_level, find_implementation, quteproc.set_setting('hints.find_implementation', find_implementation) quteproc.send_cmd(':zoom {}'.format(zoom_level)) # follow hint - quteproc.send_cmd(':hint links normal') + if 'button' in test_name: + # We are hinting buttons, link hinting will not work + quteproc.send_cmd(':hint all normal') + else: + quteproc.send_cmd(':hint links normal') quteproc.wait_for(message='hints: a', category='hints') quteproc.send_cmd(':follow-hint a') quteproc.wait_for_load_finished('data/' + parsed.target) From 012e63520f5ba90b9ab8c91754aa160010825954 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Mon, 18 Dec 2017 18:20:25 -0800 Subject: [PATCH 12/18] Blacklist non-implemented qtwebkit frame features --- tests/end2end/features/hints.feature | 3 ++- tests/end2end/features/search.feature | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index f575d0423..5da03a085 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -212,7 +212,7 @@ Feature: Using hints Scenario: Using :follow-hint inside an iframe button When I open data/hints/iframe_button.html And I hint with args "all normal" and follow s - Then "navigation request: url http://localhost:*/data/hello.txt, type NavigationTypeLinkClicked, *" should be logged + Then "navigation request: url http://localhost:*/data/hello.txt, *" should be logged Scenario: Hinting inputs in an iframe without type When I open data/hints/iframe_input.html @@ -241,6 +241,7 @@ Feature: Using hints And I run :tab-only And I hint with args "links tab" and follow a And I wait until data/hello.txt is loaded + And I wait 0.5s Then the following tabs should be open: - data/hints/iframe_target.html - data/hello.txt (active) diff --git a/tests/end2end/features/search.feature b/tests/end2end/features/search.feature index 483bcc29f..7779ff28e 100644 --- a/tests/end2end/features/search.feature +++ b/tests/end2end/features/search.feature @@ -239,6 +239,7 @@ Feature: Searching on a page - data/search.html - data/hello.txt (active) + @qtwebkit_skip: Not supported in qtwebkit Scenario: Follow a searched link in an iframe When I open data/iframe_search.html And I run :tab-only @@ -247,6 +248,7 @@ Feature: Searching on a page And I run :follow-selected Then "navigation request: url http://localhost:*/data/hello.txt, type NavigationTypeLinkClicked, is_main_frame False" should be logged + @qtwebkit_skip: Not supported in qtwebkit Scenario: Follow a tabbed searched link in an iframe When I open data/iframe_search.html And I run :tab-only From 8500509532133a55130f4f40ca1baace0f6a97f7 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Sat, 6 Jan 2018 11:13:54 -0800 Subject: [PATCH 13/18] Implement is_visible for same-origin frames --- qutebrowser/javascript/webelem.js | 33 +++++++++++++++++++------------ 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 12459f148..c236a5b0a 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -55,6 +55,18 @@ window._qutebrowser.webelem = (function() { return frame.frameElement.getBoundingClientRect(); } + // Add an offset rect to a base rect, for use with frames + function add_offset_rect(base, offset) { + return { + "top": base.top + offset.top, + "left": base.left + offset.left, + "bottom": base.bottom + offset.top, + "right": base.right + offset.left, + "height": base.height, + "width": base.width, + }; + } + function get_caret_position(elem, frame) { // With older Chromium versions (and QtWebKit), InvalidStateError will // be thrown if elem doesn't have selectionStart. @@ -126,14 +138,9 @@ window._qutebrowser.webelem = (function() { for (let k = 0; k < client_rects.length; ++k) { const rect = client_rects[k]; - out.rects.push({ - "top": rect.top + frame_offset_rect.top, - "right": rect.right + frame_offset_rect.right, - "bottom": rect.bottom + frame_offset_rect.bottom, - "left": rect.left + frame_offset_rect.left, - "height": rect.height, - "width": rect.width, - }); + out.rects.push( + add_offset_rect(rect, frame_offset_rect) + ); } // console.log(JSON.stringify(out)); @@ -141,9 +148,7 @@ window._qutebrowser.webelem = (function() { return out; } - function is_visible(elem) { - // FIXME:qtwebengine Handle frames and iframes - + function is_visible(elem, frame = null) { // Adopted from vimperator: // https://github.com/vimperator/vimperator-labs/blob/vimperator-3.14.0/common/content/hints.js#L259-L285 // FIXME:qtwebengine we might need something more sophisticated like @@ -151,7 +156,8 @@ window._qutebrowser.webelem = (function() { // https://github.com/1995eaton/chromium-vim/blob/1.2.85/content_scripts/dom.js#L74-L134 const win = elem.ownerDocument.defaultView; - let rect = elem.getBoundingClientRect(); + const offset_rect = get_frame_offset(frame); + let rect = add_offset_rect(elem.getBoundingClientRect(), offset_rect); if (!rect || rect.top > window.innerHeight || @@ -212,7 +218,8 @@ window._qutebrowser.webelem = (function() { const subelems = frame.document. querySelectorAll(selector); for (let elem_num = 0; elem_num < subelems.length; ++elem_num) { - if (!only_visible || is_visible(subelems[elem_num])) { + if (!only_visible || + is_visible(subelems[elem_num], frame)) { out.push(serialize_elem(subelems[elem_num], frame)); } } From c5e688f26c825f1ff15d2f4bb154c0f457d54ff9 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Wed, 17 Jan 2018 13:08:04 -0500 Subject: [PATCH 14/18] Stop iterating over every frame to check if element is frame --- qutebrowser/javascript/webelem.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index c236a5b0a..72083a121 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -264,13 +264,10 @@ window._qutebrowser.webelem = (function() { // Check if elem is an iframe, and if so, return the result of func on it. // If no iframes match, return null function call_if_frame(elem, func) { - for (let i = 0; i < window.frames.length; ++i) { - const frame = window.frames[i]; - if (iframe_same_domain(frame)) { - if (frame.frameElement === elem) { - return func(frame); - } - } + const frame = elem.contentWindow; + // Check if elem is a frame, and if so, call func on the window + if (frame && iframe_same_domain(frame) && frame.frameElement) { + return func(frame); } return null; } From ffda82170d32d525f44bf0a3e4ac64e706b527fa Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Wed, 17 Jan 2018 17:02:53 -0500 Subject: [PATCH 15/18] Fix several style issues --- qutebrowser/javascript/webelem.js | 12 ++++++------ tests/end2end/data/hints/html/wrapped.html | 4 +--- tests/end2end/data/hints/html/wrapped_button.html | 6 ++---- tests/end2end/data/hints/iframe_button.html | 2 +- tests/end2end/data/hints/iframe_input.html | 2 +- tests/end2end/data/iframe_search.html | 2 +- 6 files changed, 12 insertions(+), 16 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 72083a121..c4da4031d 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -285,10 +285,10 @@ window._qutebrowser.webelem = (function() { const frame_elem = call_if_frame(elem, (frame) => serialize_elem(frame.document.activeElement, frame)); - if (frame_elem === null) { - return serialize_elem(elem); + if (frame_elem !== null) { + return frame_elem; } - return frame_elem; + return serialize_elem(elem); }; funcs.find_at_pos = (x, y) => { @@ -306,10 +306,10 @@ window._qutebrowser.webelem = (function() { y - frame_offset_rect.top), frame); }); - if (frame_elem === null) { - return serialize_elem(elem); + if (frame_elem !== null) { + return frame_elem; } - return frame_elem; + return serialize_elem(elem); }; // Function for returning a selection to python (so we can click it) diff --git a/tests/end2end/data/hints/html/wrapped.html b/tests/end2end/data/hints/html/wrapped.html index 2af1a28c1..dcc05c8c7 100644 --- a/tests/end2end/data/hints/html/wrapped.html +++ b/tests/end2end/data/hints/html/wrapped.html @@ -1,8 +1,6 @@ - + diff --git a/tests/end2end/data/hints/html/wrapped_button.html b/tests/end2end/data/hints/html/wrapped_button.html index dc96bd1d5..4d5fd81bb 100644 --- a/tests/end2end/data/hints/html/wrapped_button.html +++ b/tests/end2end/data/hints/html/wrapped_button.html @@ -1,13 +1,11 @@ - + - Link wrapped across multiple lines + Button link wrapped across multiple lines
diff --git a/tests/end2end/data/hints/iframe_button.html b/tests/end2end/data/hints/iframe_button.html index cd7bf203c..610ed4c88 100644 --- a/tests/end2end/data/hints/iframe_button.html +++ b/tests/end2end/data/hints/iframe_button.html @@ -3,7 +3,7 @@ - Hinting inside an iframe + Hinting a button inside an iframe diff --git a/tests/end2end/data/hints/iframe_input.html b/tests/end2end/data/hints/iframe_input.html index 143577747..903e9a6ee 100644 --- a/tests/end2end/data/hints/iframe_input.html +++ b/tests/end2end/data/hints/iframe_input.html @@ -3,7 +3,7 @@ - Hinting inside an iframe + Hinting an input field inside an iframe diff --git a/tests/end2end/data/iframe_search.html b/tests/end2end/data/iframe_search.html index a06c0ef3e..78f388eeb 100644 --- a/tests/end2end/data/iframe_search.html +++ b/tests/end2end/data/iframe_search.html @@ -3,7 +3,7 @@ - Hinting inside an iframe + Searching inside an iframe From 968367b042c95a4aea152b5a2bba518ef76ef83b Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Fri, 19 Jan 2018 15:25:03 -0500 Subject: [PATCH 16/18] Simplify logic for checking if an element is a frame --- qutebrowser/javascript/webelem.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 76296acaa..d635de412 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -278,10 +278,13 @@ window._qutebrowser.webelem = (function() { // Check if elem is an iframe, and if so, return the result of func on it. // If no iframes match, return null function call_if_frame(elem, func) { - const frame = elem.contentWindow; // Check if elem is a frame, and if so, call func on the window - if (frame && iframe_same_domain(frame) && frame.frameElement) { - return func(frame); + if ("contentWindow" in elem) { + const frame = elem.contentWindow; + if (iframe_same_domain(frame) && + "frameElement" in elem.contentWindow) { + return func(frame); + } } return null; } From 0bdee1e2920b3a905205b5cf5f4ee4f0cc83f46f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 7 Feb 2018 18:05:15 +0100 Subject: [PATCH 17/18] Stabilize the flaky iframe test The test above this one loads hello.txt, but we don't wait for the "load finished" message, so it can arrive after the previous test already finished and make this test not wait properly. However, we also can't easily wait for the load finished message in the previous test as it only appears with QtWebEngine, not QtWebKit. As a workaround, we simply load another file in that test, to circumvent this kind of cross-interaction. --- tests/end2end/data/hints/iframe_target.html | 1 + tests/end2end/features/hints.feature | 7 +++---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/end2end/data/hints/iframe_target.html b/tests/end2end/data/hints/iframe_target.html index db8afad27..bd1e281c2 100644 --- a/tests/end2end/data/hints/iframe_target.html +++ b/tests/end2end/data/hints/iframe_target.html @@ -9,6 +9,7 @@

A link to be opened in the iframe above. + A second link for the iframe.

diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index 5da03a085..909eeaf07 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -239,12 +239,11 @@ Feature: Using hints Scenario: Opening a link with specific target frame in a new tab When I open data/hints/iframe_target.html And I run :tab-only - And I hint with args "links tab" and follow a - And I wait until data/hello.txt is loaded - And I wait 0.5s + And I hint with args "links tab" and follow s + And I wait until data/hello2.txt is loaded Then the following tabs should be open: - data/hints/iframe_target.html - - data/hello.txt (active) + - data/hello2.txt (active) Scenario: Clicking on iframe with :hint all current When I open data/hints/iframe.html From 1c662ae94c53ad7337c396d704b8482df9bd74cf Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 7 Feb 2018 18:25:25 +0100 Subject: [PATCH 18/18] Revive iframe test as flaky See #1525 --- tests/end2end/features/hints.feature | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index 909eeaf07..dc14750e1 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -222,13 +222,12 @@ Feature: Using hints # The actual check is already done above Then no crash should happen - ### FIXME currenly skipped, see https://github.com/qutebrowser/qutebrowser/issues/1525 - @xfail_norun + @flaky # FIXME https://github.com/qutebrowser/qutebrowser/issues/1525 Scenario: Using :follow-hint inside a scrolled iframe When I open data/hints/iframe_scroll.html And I hint with args "all normal" and follow a And I run :scroll bottom - And I hint wht args "links normal" and follow a + And I hint with args "links normal" and follow a Then "navigation request: url http://localhost:*/data/hello2.txt, type NavigationTypeLinkClicked, *" should be logged Scenario: Opening a link inside a specific iframe