The default bug view has changed. See this FAQ.

window-modal prompts used instead of tab-modal prompts when reentering a page using back/forward

RESOLVED FIXED in mozilla6

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Dan Wolff, Assigned: Dolske)

Tracking

(Blocks: 1 bug)

Trunk
mozilla6
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b12) Gecko/20100101 Firefox/4.0b12
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b12) Gecko/20100101 Firefox/4.0b12

Reentering a page using the back button causes the new content-modal dialogs for the JavaScript alert() and confirm() functions to be blocked, apparently defaulting back to the previous window-modal dialogs.

Reproducible: Always

Steps to Reproduce:
1. Go to a web page which uses alert() or confirm() (e.g. http://www.tizag.com/javascriptT/javascriptconfirm.php)
2. Go to another web page
3. Go back to the original page (e.g. by pressing backspace)
4. Trigger the alert() or confirm()
Actual Results:  
A window-modal dialog opens.

Expected Results:  
The content-/tab-modal dialog should open.

Comment 1

6 years ago
I can reproduce using Minefield on Linux using my main profile, but can't reproduce it in a new profile on Linux or Windows.

More precise STR (especially because I have backspace not as back):
1: go to http://www.tizag.com/javascriptT/javascriptconfirm.php
2: scroll down a bit and click the button labeled "Leave Tizag.com"
3: click ok to the tab-modal prompts twice
4: script sends you to Google
5: click back and repeat step 2
6: prompts are now the old modal dialogs, instead of tab-modal prompts
Blocks: 59314
Version: unspecified → Trunk

Comment 2

6 years ago
Dupe of bug 629911?

Comment 3

6 years ago
(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.

Updated

6 years ago
OS: Linux → All

Comment 4

6 years ago
Created attachment 515510 [details]
Minimal content policy extension for testing

I can confirm this issue. However, it has nothing to do with Adblock Plus - merely with a presence of a JavaScript-based content policy. The same thing is happening with my minimal content policy extension as well, I attached it to this bug. Unlike Adblock Plus this extension doesn't do anything other than registering a content policy and dumping incoming data to console.

Updated

6 years ago
Blocks: 467520

Comment 5

6 years ago
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

Comment 7

6 years ago
Created attachment 515575 [details]
Minimal content policy extension for testing

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

Comment 8

6 years ago
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
Component: General → General
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → general
Summary: JavaScript alert() and confirm() open modal windows when reentering a page using back/forward → 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
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

Comment 10

6 years ago
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

Comment 11

6 years ago
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.

Updated

6 years ago
(Assignee)

Comment 12

6 years ago
(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.
(Assignee)

Comment 13

6 years ago
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...
(Assignee)

Comment 15

6 years ago
Created attachment 515826 [details] [diff] [review]
Patch v.1

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+
(Assignee)

Updated

6 years ago
Duplicate of this bug: 647577
(Assignee)

Updated

6 years ago
Duplicate of this bug: 648236
Created attachment 525148 [details]
testcase

I'm seeing it also in this case, when adding an alert in a pageshow event. I guess the patch will fix the issue.
Duplicate of this bug: 650301
Why is this patch not landed yet?
status2.0: --- → ?
Keywords: checkin-needed
Note that there is a pending review comment on the patch.  Please don't check it in without addressing that comment.
(Assignee)

Comment 24

6 years ago
Exactly what I was going to say when you midaired me! :)
Keywords: checkin-needed
(Assignee)

Updated

6 years ago
Duplicate of this bug: 648627
(Assignee)

Comment 26

6 years ago
Created attachment 526444 [details] [diff] [review]
Patch v.2

Moved mHidden reset from ::Show() to ::Open(), still fixes bug.
Attachment #515575 - Attachment is obsolete: true
Attachment #515826 - Attachment is obsolete: true
(Assignee)

Comment 27

6 years ago
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
(Assignee)

Comment 28

6 years ago
Pushed http://hg.mozilla.org/mozilla-central/rev/4aaf0034ce82
Status: NEW → RESOLVED
Last Resolved: 6 years ago
status2.0: ? → ---
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
(Assignee)

Updated

6 years ago
Duplicate of this bug: 673099

Comment 30

6 years ago
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.