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

Stylish does not affect dynamically generated iframes #3

Closed
JasonBarnabe opened this issue Sep 12, 2013 · 33 comments · Fixed by #26
Closed

Stylish does not affect dynamically generated iframes #3

JasonBarnabe opened this issue Sep 12, 2013 · 33 comments · Fixed by #26
Labels

Comments

@JasonBarnabe
Copy link
Contributor Author

https://developer.chrome.com/extensions/content_scripts - match_about_blank - available in Chrome 37.

@jameson5555
Copy link

+1

The new Google Tag Manager debug iframe style is pretty hideous. I'd love to Stylish it up.

@silverwind
Copy link
Contributor

@JasonBarnabe anything else that needs to be done here other than adding match_about_blank to the permissions in manifest.json? I could send you a PR, but it's a trivial change :)

@JasonBarnabe
Copy link
Contributor Author

I'm unsure. Why don't you test it out and if that's all that's needed, send a PR.

@silverwind
Copy link
Contributor

I'll test it.

@silverwind
Copy link
Contributor

It appears it's all that was needed. Here's the iframe after the fix (was white before):

iframe

@Mottie
Copy link

Mottie commented Jan 29, 2015

Awesome news! 👍

JasonBarnabe added a commit that referenced this issue Jan 29, 2015
Add match_about_blank to fix #3
@JasonBarnabe
Copy link
Contributor Author

I tested this a bit myself. It makes things better, but still not great.

-Testing on Tumblr's new post box, it still doesn't work. This is likely because the src is javascript:"" rather than nothing (about:blank).
-Testing on MDN's editor, it works if you apply the style after the page has loaded, but not if you load the page with the style enabled already. Maybe this is because the iframe's not there when Stylish runs?

@JasonBarnabe JasonBarnabe reopened this Jan 29, 2015
@silverwind
Copy link
Contributor

For the first case: I think we need to change the patterns. Will experiment with javascript urls, file:// is also missing there.

For the second case: Maybe something like a mutation observer is needed to re-apply when iframes get added. But there's probably a cleaner solutions out there.

@JasonBarnabe
Copy link
Contributor Author

For the first case: I think we need to change the patterns. Will experiment with javascript urls, file:// is also missing there.

Chrome's docs say javascript: is not allowed. I've added file: in #25.

@silverwind
Copy link
Contributor

Chrome's docs say javascript: is not allowed

Can you link me to where you read that? Sounds like another Chromium issue is required.

@JasonBarnabe
Copy link
Contributor Author

Same doc you linked to. It lists allowed schemes, and javascript: isn't one of them.

A more likely solution is extending match_about_blank to javascript: URLs, though.

@silverwind
Copy link
Contributor

Well, I don't see javascript: mentioned explicitly, so I think <all_urls> or * might be worth a try.

@silverwind
Copy link
Contributor

No luck with <all_urls> and * is not a valid entry. My iframe in question (on irccloud.com) doesn't have a src attribute, but once I added github.com to the matched domains of the style, it did work. Pretty strange nonetheless.

@silverwind
Copy link
Contributor

@JasonBarnabe I tried your MDN example, but it works as expected on page reloads. Here's the style I was using:

@namespace url(http://www.w3.org/1999/xhtml);

@-moz-document domain("developer.mozilla.org") {
  .cke_wysiwyg_frame, .cke_wysiwyg_div { margin-left: 50% !important;}
}

This is on Chrome 42. Could you verify again that the MDN case works for you?

Also, please point me to the iframe you meant on tumblr. I could only identify a few hidden ones in the HMTL. Maybe <all_urls> could solve that case.

@silverwind
Copy link
Contributor

Noticed the page reload issue on another site. I think this is only fixable through JS.

@JasonBarnabe
Copy link
Contributor Author

Tumblr must've changed their editor since I tried last, because yeah, there's no iframe involved any more.

@hideheader
Copy link
Contributor

(1) Tumblr dropped TinyMCE and began using contentEditable elements on Jan 28. The TinyMCE samples use the same pattern. Chrome doesn't load content scripts into src='javascript:""' iframes - the match_about_blank fix will always fail on these frames.

(2) The intermittent failure in MDN's editor occurs when Chrome is able to run the content script before the page writes content to the iframe. A minimal default document, <html><head/><body/></html>, is created in the iframe when it's inserted into the DOM. If the page script that writes the iframe content runs before the Stylish content script, you see styles. If the content script runs first, Stylish adds styles to the default document, which are subsequently lost when the page replaces the iframe content. It's a race.

The only remedy for intermittent failures like this is to monitor the styles with a MutationObserver and replace them.

(3) Here are some test cases. Stylish is yes if Chrome ran the content script, styled is yes if the style was applied, src is <iframe>.src and docURI is <iframe>.contentDocument.documentURI.

src= Stylish styled src docURI
1 none yes yes http://ace.c9.io/ http://ace.c9.io/#nav=embedding
2 '' yes yes http://ace.c9.io/ http://ace.c9.io/#nav=embedding
3 javascript:"" no no javascript:"" http://ace.c9.io/#nav=embedding
4 data:... yes yes data:... http://ace.c9.io/#nav=embedding
5 srcdoc yes no '' about:srcdoc
6 none [†] yes no '' about:blank
7 gardez:l.eau yes yes gardez:l.eau http://ace.c9.io/#nav=embedding
8 # yes yes http://ace.c9.io/# http://ace.c9.io/#nav=embedding

[†] createHTMLDocument(), then replaceChild()

// All but cases 5 & 6 set a `src` URL, then write the `contentDocument`
l = document.body.appendChild(document.createElement('iframe'));
l.src = aString; // omitted, if aString is none
d = l.contentDocument; d.open(); d.write(iframeContent); d.close();

// Case 5 sets `srcdoc` to the document content instead of writing `contentDocument`
l = document.body.appendChild(document.createElement('iframe'));
l.srcdoc = iframeContent;

// Case 6 creates a complete `document` de novo, then inserts it into the `iframe`
d=document.implementation.createHTMLDocument('Editor IFRAME')
d.open(); d.write(iframeContent); d.close();

l = document.body.appendChild(document.createElement('iframe'));
d2 = l.contentDocument;
d2.replaceChild(d2.adoptNode(d.documentElement), d2.documentElement);

(4) As you can see,
Cases 1-6: half fail
match_about_blank really isn't a solution. What's needed is a MutationObserver that copies existing styles from into new frames, another that copies new styles into existing frames, and a third that detects when styles are removed and replaces them when appropriate. The first two are easy. The third requires a little experimentation, since you don't want to reinsert them while the page is being torn down.

@silverwind
Copy link
Contributor

Just wanted to say thank you @hideheader, awesome work!

@hideheader
Copy link
Contributor

This pull request is a pair of MutationObservers that copy Stylish STYLE elements from the top-level document into child IFRAMEs. I've tested with the examples above using the Ace editor, on the TinyMCE editor, and on MDN's editor using this style. It works with all but srcdoc='<html/>' frames, which load contentDocument very late; those can be handled in onLoad on the IFRAME.

The PR only adds styles when the IFRAME is added to the DOM, just as apply.js does, so there should still be intermittent failures with MDN's editor. Unfortunately it hasn't been failing on MDN, so I need an example page that overwrites frame content after the content script has loaded.

Both IFRAME.onload and webNavigation.onCommitted fire when frame content is reloaded. Each has issues; test cases are needed to sort them out.

If you want to experiment, the penultimate commit before the PR can add a MutationObserver on HTML and HTML > *, and an onLoad handler on IFRAMES; localStorage['observer.observeFrameContent'] = localStorage['observer.observeFrameLoad'] = 'true' enables them.

@JasonBarnabe
Copy link
Contributor Author

Committed a fix to the iframe mutation observer. Evidentally the test page I stole from @hideheader never actually triggered this functionality.

More testing is welcome.

@hideheader
Copy link
Contributor

My, that was lucky. If you hadn't refactored the code and broken it we still wouldn't know that there's a blind spot in the test cases. What condition is it that wasn't being triggered?

@JasonBarnabe
Copy link
Contributor Author

The iframe mutation observer never ran; I assume because the iframes were added before the content script ran. So everything appeared to work, whether loading the page with the style on or turning it on after load, but only the page load logic was being exercised.

@JasonBarnabe
Copy link
Contributor Author

Fixed the pushState case for feedly.

@tophf
Copy link
Contributor

tophf commented May 27, 2015

Fixed, evidently, may be closed.

@YodaEmbedding
Copy link

YodaEmbedding commented Jun 19, 2016

Still appears not to work with the following URL: https://mail.google.com/tasks/canvas?pli=1

Test case:

@namespace url(http://www.w3.org/1999/xhtml);

@-moz-document url-prefix("https://mail.google.com/tasks/canvas") {

  .J {
    background-color: #333 !important;
  }

  .Z {
    color: #ccc !important;
  }

  .o, .k, .p, .i, .Mb {
    border-color: #444 !important;
  }

}

@Mottie
Copy link

Mottie commented Jun 20, 2016

@SicariusNoctis Google changes element class names with every new page load. Target elements using a nodeName and DOM structure (e.g. body > div > table).

@YodaEmbedding
Copy link

I don't think that's the issue here: the class names stay consistent, or at the very least, every time I refresh the page. The problem is, they are inside an iframe, and there appears to be no way to target them.

@tophf
Copy link
Contributor

tophf commented Jun 21, 2016

Still appears not to work with the following URL: https://mail.google.com/tasks/canvas?pli=1

Can't reproduce on Chrome 53.0.2767.4 dev-m.

With the style applied:
2-fs8

Without:
1-fs8

@YodaEmbedding
Copy link

YodaEmbedding commented Jun 22, 2016

It is not working on Firefox 47.0 (release) running Stylish 2.0.6 (release).

EDIT: It does work on Google Chrome, however.

@tophf
Copy link
Contributor

tophf commented Jun 22, 2016

There's a separate issue #166 as Firefox's Chrome-compatible WebExtensions API is still incomplete.

@j-ulrich
Copy link

j-ulrich commented Jun 28, 2017

Still doesn't work for me with a src="javascript:""" iframe in Chrome 58 (64-bit, Win7).
Unfortunately, I cannot link to the page because it's not publicly accessible. But it's a Confluence page being edited.
Am I missing something? Do I have to add "about:blank" to the URL matching rules of the design or something like that?

@tophf
Copy link
Contributor

tophf commented Jun 28, 2017

@j-ulrich, Stylish seems to be on indefinite hiatus. Try Stylus instead.

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

Successfully merging a pull request may close this issue.

8 participants