Issue when closing sidebar (ex bookmarks) #1298

Closed
GM-Script-Writer-62850 opened this Issue Mar 1, 2011 · 9 comments

Projects

None yet

4 participants

@GM-Script-Writer-62850

Browser: Mozilla/5.0 (X11; Linux i686; rv:2.0b13pre) Gecko/20110301 Firefox/4.0b13pre ID:20110301030403
GM: 2011.01.24.nightly
OS: Ubuntu 10.04.2 64bit/xubuntu 10.04.2 32bit

I have verified this happens with greasemonkey
There is no issue in safe mode
It occurred with the only enabled add-on being greasemoneky

Steps to reproduce:
Open sidebar [ctrl]+[B]
Close sidebar via close button at the top right of the side bar
Watch CPU usage skyrocket
it may take a minute for Firefox to become unresponsive
When I close the sidebar the CPU usage goes to 100% and will lockup eventually

@GM-Script-Writer-62850

It did not happen with all scripts off
it did happen with only one script enabled http://userscripts.org/scripts/show/45517

@arantius
Collaborator

Confirmed, Mozilla/5.0 (X11; Linux i686 on x86_64; rv:2.0) Gecko/20100101 Firefox/4.0, GM head.

@arantius
Collaborator

Confirmed only with that script running, not with the handful of others I tried. Tracing shows that GM_BrowserUI.contentLoad() is being repeatedly called.

@arantius
Collaborator

At least I've got a reduced test case: https://gist.github.com/920262

Since #1038 / 1f0efae we've allowed injection into about:blank. This might be a bad idea. In this case I believe it's causing infinite recursion: Script loads in a page, creates a frame (no src, so about:blank) which causes a(n empty) document to load, which the script runs in, where it creates a frame, which ..

https://bugzilla.mozilla.org/show_bug.cgi?id=136580
Firefox will not nest a URL inside itself normally, so this only happens once in normal content. Something weird is going on with the sidebar. When it is closed, the src is set to about:blank, and this nesting logic does not kick in, so the iframes get nested infinitely.

It must be the case that the top-level node (with src=about:blank?) is considered part of chrome and doesn't get this limit. There's also a maximum nesting (regardless of URL) limit of 10, but it's specifically called "MAX_DEPTH_CONTENT_FRAMES" -- if the browser doesn't think that it's content, it's probably not applying this limit.

Mayhaps we should stop injecting into about:blank?

@GM-Script-Writer-62850

idea what about only allowing about:blank if it is specifically specified in the scripts metadata
possible new rule: wild cards will not include about:blank bur you can include about:blank using @include about:blank

@kwah
kwah commented Apr 14, 2011

I would perhaps go one further and deprecate @include * completely, and as a stop-gap, infer it to mean http://* and https://

If instead you do wish to include it everywhere, you must explicitly specify the protocols you wish it to apply to:
@include http://*
@include https://*
@include ftp://*
@include ftps://*
@include about:*
@include file://*

etc..

I'd suggest that this adds clarity for free too - it doesn't apply so much with the fileIsGreaseable and aboutIsGreaseable flags - but will make it more obvious to the user which kinds of sites the script will run on (to the non-greasemonkey initiated, @include * is probably quite cryptic).

@GM-Script-Writer-62850

i would think anyone who has heard the term wild card and is not referring to a card game would understand *

@sizzlemctwizzle
I would perhaps go one further and deprecate @include \* completely
See #1016 and @match.
@kwah
kwah commented Apr 14, 2011

i would think anyone who has heard the term wild card and is not referring to a card game would understand *

Too many assumptions - you're assuming that @include is understood as referring to the urls that the script runs on and you're assuming that * will instantly be recognised as a wildcard.

I don't dispute that given a push the meaning of @include * could be squeezed out of a good portion of users, but especially if you were to consider non-technical & non-coder users specifically, I would suggest that wildcards aren't immediately obvious unless they're in context (eg at the end of a string or mid-string as opposed to completely on its own).

@arantius arantius added a commit to arantius/greasemonkey that referenced this issue Apr 20, 2011
@arantius arantius Do not run scripts in about:blank
(Unless _specifically_ requested.)  Fixes #1298
84d4a9e
@arantius arantius added a commit that closed this issue Apr 20, 2011
@arantius arantius Do not run scripts in about:blank
(Unless _specifically_ requested.)  Fixes #1298
84d4a9e
@arantius arantius closed this in 84d4a9e Apr 20, 2011
@arantius arantius reopened this Apr 22, 2011
@arantius arantius closed this Apr 22, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment