Closed Bug 518327 Opened 15 years ago Closed 15 years ago

"Allow Pop-ups for ..." menu item not enabled again when leaving Private Browsing mode

Categories

(Firefox :: Private Browsing, defect)

Product:
Firefox
Component:
Private Browsing
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 3.7a1
Tracking Flags:
Tracking Status
status1.9.2 --- .4-fixed
status1.9.1 --- .10-fixed

People

(Reporter: whimboo, Assigned: ehsan.akhgari)

References

Details

(Keywords: regression, verified1.9.1, verified1.9.2)

Attachments

(2 files, 1 obsolete file)

Patch (v1)
(deleted), patch
mconnor
: review+
beltzner
: approval1.9.2.2-
Details | Diff | Splinter Review
Rolled up patch
(deleted), patch
beltzner
: approval1.9.2.4+
beltzner
: approval1.9.1.10+
Details | Diff | Splinter Review
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20090922 Namoroka/3.6b1pre When you leave the Private Browsing mode the "Allow pop-ups for ..." menu item is not enabled again. It still resists in the disabled state. It's a regression from bug 461625. Steps: 1. Start the Private Browsing mode 2. Open http://popuptest.com/popuptest1.html and click on the Options/Preferences button 3. Stop the Private Browsing mode 4. Do step 2 again Now the menu entry is still disabled. Should happen because of the following two lines of code: if (gPrivateBrowsingUI.privateBrowsingEnabled) blockedPopupAllowSite.setAttribute("disabled", "true");
Attached patch Patch (v1) (obsolete) (deleted) — — Splinter Review
Fix + unit test (this should be applied on top of attachment 404350 [details] [diff] [review] from bug 461625).
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
Attachment #404351 - Flags: review?(mconnor)
This looks like the fix for the pb toggle... wrong patch?
Attached patch Patch (v1) (deleted) — — Splinter Review
Yes, sorry, I had attached the wrong patch here.
Attachment #404351 - Attachment is obsolete: true
Attachment #418185 - Flags: review?(mconnor)
Attachment #404351 - Flags: review?(mconnor)
Comment on attachment 418185 [details] [diff] [review] Patch (v1) Wicked/
Attachment #418185 - Flags: review?(mconnor) → review+
http://hg.mozilla.org/mozilla-central/rev/0d1a015fbabe
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
Attachment #418185 - Flags: approval1.9.2.1?
Comment on attachment 418185 [details] [diff] [review] Patch (v1) >+ blockedPopupAllowSite.removeAttribute("disabled", "true"); Doesn't need second argument?
(In reply to comment #6) > (From update of attachment 418185 [details] [diff] [review]) > > >+ blockedPopupAllowSite.removeAttribute("disabled", "true"); > > Doesn't need second argument? Oh, you're totally right. Fixed in: http://hg.mozilla.org/mozilla-central/rev/1bd11edbec1f.
Comment on attachment 418185 [details] [diff] [review] Patch (v1) Need a rollup patch of both changesets.
Attachment #418185 - Flags: approval1.9.2.2? → approval1.9.2.2-
If this is worth fixing on stable branches we probably want it in 1.9.1.x too.
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Attached patch Rolled up patch (deleted) — — Splinter Review
Attachment #433160 - Flags: approval1.9.2.3?
Attachment #433160 - Flags: approval1.9.1.10?
Comment on attachment 433160 [details] [diff] [review] Rolled up patch a=beltzner for 1.9.1.10 and 1.9.2.3 - thanks, Ehsan!
Attachment #433160 - Flags: approval1.9.2.3?
Attachment #433160 - Flags: approval1.9.2.3+
Attachment #433160 - Flags: approval1.9.1.10?
Attachment #433160 - Flags: approval1.9.1.10+
Verified fixed for 1.9.2 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4pre) Gecko/20100412 Namoroka/3.6.4pre (.NET CLR 3.5.30729). Verified for 1.9.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.10pre) Gecko/20100412 Shiretoko/3.5.10pre (.NET CLR 3.5.30729).
Keywords: verified1.9.1, verified1.9.2

Hello. I am confused about this STR. It is included in a test case and something is preventing me from validating it.

Steps I used to verify behavior:

  1. Start the browser normally.
  2. Open a Private Browsing window.
  3. Open http://popuptest.com/popuptest1.html in the Private Browsing window.
    Observe: the page is opened and a yellow notification bar is displayed: "Nightly prevented this site from opening 6 pop-up windows."
  4. Click on the Options/Preferences button (from the notification bar? from the hamburger menu? and do what?)
  5. Stop the Private Browsing mode (close the private browsing window to remain with the non-private browsing window?)
  6. Do steps 3 and 4 again in the non-private browsing window.

Now the menu entry is still disabled. (Whan do you mean?)

Could you please clarify these confusions? Thanks.

Flags: needinfo?(hskupin)
Flags: needinfo?(ehsan)

What are you trying to validate? This bug is 10 years and and was fixed and verified 9 years ago.

Flags: needinfo?(ehsan)
Flags: needinfo?(hskupin) → needinfo?(daniel.bodea)

I am validating Nightly and I stumbled upon an old test case. Help me correct it. Should it be removed?

Flags: needinfo?(daniel.bodea) → needinfo?(hskupin)

I don't know. Maybe Ehsan can help instead.

Flags: needinfo?(hskupin) → needinfo?(ehsan)

I'm sorry, I don't understand where the STR in comment 14 originally comes from, and who wrote it and who these questions are targeted at. I also do not understand why we are trying to verify a 9 year old bug or looking at tests for it.

At any rate, years after this bug was fixed we completely rewrote private browsing mode from scratch to make it work on a per-window basis rather than it being an application-wide global mode that the user could be in or out of. The STR in comment 14 is referring to that outdated implementation. It has been enough years that I don't remember how many years it's been since I've even thought about it.

No further action of any kind (discussions/tests/QA/verification/etc) is necessary for this bug. Thanks!

Flags: needinfo?(ehsan)

Thank you.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: