Closed Bug 637160 Opened 11 years ago Closed 10 years ago
window-modal prompts used instead of tab-modal prompts when reentering a page using back/forward
Version: unspecified → Trunk
Dupe of bug 629911?
(In reply to comment #2) This looks nothing like that. I've managed to reproduce this on Linux and Windows starting from a new profile now. The missing ingredient: install Adblock Plus 1.3.3 CCing Wladimir. I'm worried that somehow Adblock might accidentally be causing web JS to run in a different context that gets out of the tab-modal prompts. Worst-case scenario would be if it causes it to be run with chrome privileges. Notes: Also tried with latest Adblock Plus dev build with same results. Used Easylist subscription with tests.
Content policies was were I would expect the problem to be coming from. I can't seem to reproduce it using the attached test extension, however. Anything extra I need to do besides installing it into a new profile?
Works For Me with a clean profile on Build identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12 Works For Me with Adblock Plus 1.3.3 installed on Build identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12
My bad, I forgot that the content policy extension in my profile was a modified version. Attached the modified version now, verified that it will cause the issue on a clean profile (Minefield build 20110227 on Windows 7). It seems that having a content policy is not enough, one also has to block a script on the page - and that's what this extension is doing now (blocking http://pagead2.googlesyndication.com/pagead/show_ads.js request).
Attachment #515510 - Attachment is obsolete: true
Ah, ok. Now I can reproduce using your second testcase extension. Thanks. I'll confirm this and move it over to Core:General. Not sure exactly what component it belongs in, though.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → general
Justin, any idea what's going on here? Are we somehow getting confused about which tab the dialog belongs to?
Component: General → DOM
QA Contact: general → general
I reduced the content JS part of the testcase and put it in a data URI above. Note that this is reproducible with just one confirm(), or one alert(), or one prompt(). Same effect for each: navigate to another page and back and then it'll be window-modal instead of content-modal.
Summary: content policy blocking a script (e.g. Adblock Plus) can turn content-modal alert() and confirm() prompts into window-modal dialogs when reentering a page using back/forward → content policy blocking a script (e.g. Adblock Plus) can turn content-modal prompts into window-modal dialogs when reentering a page using back/forward
Just to be clear, the test extension is blocking "/show_ads.js" but the test content JS doesn't need to have the file being blocked. I can still reproduce with it with only a button for a prompt and the test extension.
(In reply to comment #9) > Justin, any idea what's going on here? Are we somehow getting confused about > which tab the dialog belongs to? Hmm. I'd actually guess this is a result of the code added in bug 613800, which forces use of window-modal prompts in some cases (ie, when a tab-modal prompt could result in unexpected reentrancy). Still seems unexpected here, though, so maybe some of the existing accounting which 613800 checks is incorrect? I can reproduce this with my normal profile on OS X (which has Adblock), but not yet on my Windows box. Lemme try again with the STR from the last few comments.
I can reproduce on Windows now. In DocumentViewerImpl::GetIsTabModalPromptAllowed(), mHidden is |true|. Which I guess isn't actually surprising, because there doesn't seem to be any code to set it back to |false| when a previous-hidden page is no longer hidden. Sigh.
Er, yeah. Looks like Show() should set mHidden to false? I wonder why that ever worked...
This fixes the problem for me. Any other places that mHidden should be reset? I don't know this code well enough to say, but this seems like the obvious missing spot. ::Stop() is the only other place mHidden is checked [other than ::GetIsTabModalPromptAllowed()], so this seems safe enough. Hmm, why do the STR need a content policy to trigger this? I don't see an obvious connection, does the policy have some effect on bfcache / when things are shown or hidden?
Assignee: nobody → dolske
Attachment #515826 - Flags: review?(bzbarsky)
> Hmm, why do the STR need a content policy to trigger this? This part I don't know. It seems like any time a page comes from bfcache we should hit this problem, no?
Comment on attachment 515826 [details] [diff] [review] Patch v.1 Does putting this in Open() also work? It would make a bit more sense there, I think... r=me either way.
Attachment #515826 - Flags: review?(bzbarsky) → review+
I'm seeing it also in this case, when adding an alert in a pageshow event. I guess the patch will fix the issue.
Note that there is a pending review comment on the patch. Please don't check it in without addressing that comment.
Exactly what I was going to say when you midaired me! :)
Moved mHidden reset from ::Show() to ::Open(), still fixes bug.
Adjusting summary, since I don't see anything to indicate a content policy is actually required to trigger this bug. Feel free to retest after this lands, and if there's still some remaining issue involving a content policy, let's file a new bug (and just refer to this one for some of the history).
Summary: content policy blocking a script (e.g. Adblock Plus) can turn content-modal prompts into window-modal dialogs when reentering a page using back/forward → window-modal prompts used instead of tab-modal prompts when reentering a page using back/forward
Seeing this in the latest 7.0 beta with AdBlock Plus installed. Can confirm that disabling ABP and reloading the page fixes the problem.
You need to log in before you can comment on or make changes to this bug.