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

Missing hints from iframe #28

Closed
richardmcmillen opened this issue Jun 29, 2022 · 19 comments
Closed

Missing hints from iframe #28

richardmcmillen opened this issue Jun 29, 2022 · 19 comments
Labels

Comments

@richardmcmillen
Copy link
Contributor

richardmcmillen commented Jun 29, 2022

I have only just seen this now and haven't dug into how when it is happening, highlighted blue area does not have hints and it is an iframe.
image

@david-tejada
Copy link
Owner

Can you check what those elements are? Maybe it doesn't have anything to do with being in an iframe.

@richardmcmillen
Copy link
Contributor Author

richardmcmillen commented Jun 29, 2022

They are anchor tags. This is the a site I use pretty frequently and it has worked for me previously I am almost certain.

<a class="awsui_link_4c84z_1bip0_93 awsui_variant-secondary_4c84z_1bip0_138 awsui_font-size-body-m_4c84z_1bip0_406"
  aria-label="Instances (running)" href="#Instances:instanceState=running">
  <div class="Panel">Instances (running)<span
      class="awsui_root_18wu0_1bzxa_93 awsui_box_18wu0_1bzxa_205 awsui_f-right_18wu0_1bzxa_995 awsui_color-inherit_18wu0_1bzxa_335 awsui_font-size-default_18wu0_1bzxa_221 awsui_font-weight-default_18wu0_1bzxa_261">78</span>
  </div>
</a>

@david-tejada
Copy link
Owner

That should work. I'll see if I can make an account and figure out what's happening

@richardmcmillen
Copy link
Contributor Author

richardmcmillen commented Jun 30, 2022 via email

@david-tejada
Copy link
Owner

Yeah, I was hoping creating an account would be an easy process. If you find the time we can pair and try to fix it

@david-tejada
Copy link
Owner

I don't think it's this but maybe you inadvertently disable hints for the host/page. Try the command hints reset everywhere to make sure is not that.

@richardmcmillen
Copy link
Contributor Author

richardmcmillen commented Jul 2, 2022

I did hints reset everywhere and yea it didn't change anything.

So I did a bit of digging and found some things...

I noticed that the bottom of the webpage there is yet another iframe (nested in the other one) which does have hints in it - the "Help topics" section.

image

Debugging on Main

So I thought I would have a go at debugging myself so spun up the latest version of main and the same webpage has even less hints. I use:

npm run-script watch
web-ext run --source-dir dist-mv2

To load the extensions in to a separate instance of Firefox And I only get the hints at the very bottom of the page:

image

Here are some observations:

  1. Errors in the console - DOMException: Document.querySelector: '.0-1656744554848-3410' is not a valid selector content.ts:53:10. This error seems legit due to weird identifier having a number first but I wonder if it is breaking further processing.

image

Yet I can't find anything element in the DOM with a class of 0-1656744554848-3410, though there is a input with this as its id

<input id="0-1656744554848-3410" aria-labelledby="0-1656744554848-3410-label" class="_input_4yi2u_db2mu_14" type="checkbox">

There is a second error in the console for the same type of issue but from a different code path:

image

  1. Scroll event isn't firing - when I scroll the main part of the page (which is actually an iframe) the scroll event doesn't fire, I saw this by putting a debugger in the scroll callback function to see if it would fire and it doesn't.

@richardmcmillen
Copy link
Contributor Author

Hmm I realise that possibly my debugger didn't fire because I had added a breakpoint to only one iframes content script, I am not sure how debuggers works and multiple iframes which run the same extension
image

I see once instance of rango for each iframe on the page of which there are about 6.

@david-tejada
Copy link
Owner

Sorry about this. I introduced bug when I created the code to filter out disabled elements' labels. I also mixed up the class with the id selector. That is fixed now, you can pull and try again.

I think the links in Resources not appearing could be a similar error that we are not catching and halts everything

@richardmcmillen
Copy link
Contributor Author

richardmcmillen commented Jul 3, 2022

No worries!! Thanks for taking a a look. Using the latest from main does fix the left hand sidebar hints 🎉 but the main page iframe remains un-hinted except for the nested iframe "Help Topics".

image

@richardmcmillen
Copy link
Contributor Author

So I can only see two #rango-hints-container's in the DOM, one for the main document, and then one for the nested iframe in iframe "Help Topics" section.

@richardmcmillen
Copy link
Contributor Author

richardmcmillen commented Jul 3, 2022

Adding this only prints 2 document elements (the two that are marked up with hints), even though there are 4 iframes on the page (more if you count nested ones.)

I would have expected this to print the document from each iframe on the page regardless of whether or not the iframe is visible or not. This has kinda left me unsure where to go from here, as I was hoping to step through the logic from the page loading, but if my understanding is correct rango isn't firing for some of these iframes.

diff --git a/src/content/intersectors.ts b/src/content/intersectors.ts
index cf76a96..ab61845 100644
--- a/src/content/intersectors.ts
+++ b/src/content/intersectors.ts
@@ -2,6 +2,7 @@ import { Intersector } from "../typing/types";
 import { getClickableType } from "./utils/clickable-type";
 import { NoHintError } from "./classes/errors";
 
+console.debug(document)
 export const intersectors: Intersector[] = [];
 export const removedIntersectorsHints: Set<string> = new Set();

@richardmcmillen
Copy link
Contributor Author

I checked out a previous version of rango just to make sure I wasn't misremembering and that this ever worked and with version v0.1.9 this page does seem to work, all the links you would expect are hinted:

image

@richardmcmillen
Copy link
Contributor Author

richardmcmillen commented Jul 3, 2022

I did a git bisect to step through commits from version v0.1.9 to v0.2.0 and it pointed to this commit which was the first commit that resulted in the main page area didn't have the hints - 2efd458

Which is weird as I can't see anything that stands out in that commit. But before this commit, it worked, with this commit applied it does not.

There are no errors in the console.

@david-tejada
Copy link
Owner

I ended up making an account and found out where the bug is. It's in this function:

export function getDefaultBackgroundColor(): Color {
// Have to add to the document in order to use getComputedStyle
const div = document.createElement("div");
document.head.append(div);
const backgroundColor = window.getComputedStyle(div).backgroundColor;
div.remove();
return new Color(backgroundColor);
}

The funny thing is that if I visit the dashboard directly the hints work but if I reload they don't. So I guess it has to do with the document not being ready when it tries to attach the element to head. Anyway, I think I'm going to scratch that code. I found it in stack overflow but I think we can assume the default color to be rgba(0, 0, 0, 0).

Thanks for your help. It would have taken me longer without it. I didn't know about git bisect, that seems really useful

@david-tejada
Copy link
Owner

david-tejada commented Jul 4, 2022

I got to the bottom of this. Sometimes backgroundColor in line 27 is an empty string. And when you pass an empty string to new Color it results in an error. You have to wrap the statement in a try catch block to see it
lun 04 jul 2022 10:56:02 CEST

@richardmcmillen
Copy link
Contributor Author

richardmcmillen commented Jul 5, 2022 via email

@david-tejada
Copy link
Owner

No problem, Amazon already had my card number so 🤷‍♂️. I'll close it if I see I'm not going to use it

Yeah, I don't know why the error didn't show up in the console. There is no error handling for that code, it's called from here:

const defaultBackgroundColor = getDefaultBackgroundColor();

@david-tejada
Copy link
Owner

So, I figured out why the exception wasn't appearing in the console. I was testing some code and I noticed there wasn't any error log for an exception that I knew was happening. The debugger would even stop if I marked "Pause on exceptions". Turns out exceptions from content scripts don't log to the Web Console in Firefox. You have to use the Browser Console if you want to see those. They log fine to the web console in Chrome, though.

1410932 - Errors raised in the content scripts context should be logged in the related tab's webconsole

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants