If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

can create lots of safebrowsing notification bars

RESOLVED FIXED in Firefox 3.7a1

Status

()

Toolkit
Safe Browsing
--
minor
RESOLVED FIXED
9 years ago
3 years ago

People

(Reporter: Samuel Sidler (old account; do not CC), Assigned: Joe Bowser)

Tracking

Trunk
Firefox 3.7a1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 2 obsolete attachments)

This happens using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b5pre) Gecko/20090511 Shiretoko/3.5b5pre

STR:
  1. Go to http://www.mozilla.com/firefox/its-an-attack.html
  2. "Ignore this warning"
  3. Go to http://www.mozilla.com/firefox/its-an-attack.html again
  4. "Ignore this warning"
  5. Dismiss the notification bar
  6. Dismiss the notification bar
  7. Wait, what?

Gavin said this is pretty easy to fix.
I guess we should just remove any existing notifications when displaying the new one:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#2361
Whiteboard: [good first bug]
Blocks: 503984
No longer blocks: 503984
(Assignee)

Comment 2

8 years ago
Created attachment 395944 [details] [diff] [review]
Simple if statement that checks if this notification already exists.

Just check to see if the current notification is not null, AND is not one that is noticibly different from the phishing bug.
Comment on attachment 395944 [details] [diff] [review]
Simple if statement that checks if this notification already exists.

Is Gavin the right reviewer here?
Attachment #395944 - Flags: review?(gavin.sharp)
I think we probably want to remove the current notification and replace it with the new one, in the case where we're changing from one site to another (or one type of block or another). You can do that with removeNotification.

Think you could  whip up a patch that does that, Joe?
Attachment #395944 - Flags: review?(gavin.sharp)
(Assignee)

Comment 5

8 years ago
Doing that now.
(Assignee)

Comment 6

8 years ago
Created attachment 396322 [details] [diff] [review]
Checks for the error and removes it if it exists, so the 2nd one has priority

This checks for the bar, removes it and places the new bar.
Attachment #395944 - Attachment is obsolete: true
(Reporter)

Updated

8 years ago
Attachment #396322 - Flags: review?(gavin.sharp)
Comment on attachment 396322 [details] [diff] [review]
Checks for the error and removes it if it exists, so the 2nd one has priority

Thanks for the patch, looks good! Just a couple of style nits:

-remove the "!= null"
-add a space after "if"
-omit the braces.

whoever checks this in can just fix that on checkin, but feel free to also attach an updated patch if you'd like to expedite the process.
Attachment #396322 - Flags: review?(gavin.sharp) → review+
(Reporter)

Updated

8 years ago
Assignee: nobody → bowserj
(Assignee)

Comment 8

8 years ago
Created attachment 396596 [details] [diff] [review]
Cleaned patch with minor style changes
Attachment #396322 - Attachment is obsolete: true
(Reporter)

Updated

8 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/e4f9310ecd0a
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
Component: Phishing Protection → Phishing Protection
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.