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

RESOLVED FIXED in mozilla6

Status

()

RESOLVED FIXED
8 years ago
a year ago

People

(Reporter: 326374, 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

8 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

8 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

8 years ago
Dupe of bug 629911?

Comment 3

8 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

8 years ago
OS: Linux → All

Comment 4

8 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

8 years ago
Blocks: 467520

Comment 5

8 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

8 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

8 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

8 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

8 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.
(Assignee)

Comment 12

8 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

8 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

8 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

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

Updated

8 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

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

Updated

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

Comment 26

8 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

8 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

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

Updated

7 years ago
Duplicate of this bug: 673099

Comment 30

7 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.

Updated

a year ago
Duplicate of this bug: 642699
You need to log in before you can comment on or make changes to this bug.