509468 - Decide what to do with "Get me out of here" button (name, action)

Status: RESOLVED FIXED

(Camino Graveyard :: Security, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

+++ This bug was initially created as a clone of Bug #480207 +++

In bug 480207 we left untouched the issue of the "Get me out of here" button.

In Firefox, it goes Home (and to "Start" if Home is a blocked site)
In Safari, the equivalent button closes the page (and is called "Close")
In Opera, the equivalent button goes Home (and is called "Go to Home Page")

We currently go Back:

* "Get me out of here" is great emotionally, it doesn't clearly indicate to the user what concretely will happen
* What happens if you arrive on a bad site via a redirect? 
* If we keep using "Get me out of here" as a button name, we should add "!" (like Firefox 3.5)

Comment 1

[Stuart Morgan]

I vote for closing the page; simple and effective.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #1)
> I vote for closing the page; simple and effective.

Per the meeting, "make it so!"

Comment 3

[Sean Murphy]

Attachment: Patch

Change to Close Page.

Comment 4

[Sean Murphy]

Actually, is it our plan to do the same for the "Get Me Out" button on the blocker bar?

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I think so.

Comment 6

[Sean Murphy]

Comment on attachment 394063 [details] [diff] [review]
Patch

Updating the patch to include the blocker bar as well.

Comment 7

[Sean Murphy]

Attachment: Patch, v2

This time includes the bar as well.  Requesting behavior and button label approval from Smokey first.

New SafeBrowsingBar.nib to follow, since I feel we should of course indicate on the button that it will close the page.

Comment 8

[Sean Murphy]

Attachment: SafeBrowsingBar.nib

Changes the get me out button title to "Close Page"

Comment 9

[Sean Murphy]

Attachment: Patch, v2 (From updated source tree)

My source tree was mistakingly out of date when generating the last patch.  Refreshed to apply successfully now.

Comment 10

[Sean Murphy]

Attachment: SafeBrowsingBar.nib (With title placeholder)

As per IRC, Updated the warning label to indicate it is just a placeholder and does not need localization in the nib.

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 394325 [details] [diff] [review]
Patch, v2 (From updated source tree)

>Index: resources/localized/English.lproj/Localizable.strings.in
>===================================================================

>-"GetMeOutButtonLabel" = "Get me out of here!";
>+"GetMeOutButtonLabel" = "Close Page";
> "IgnoreWarningButtonLabel" = "Ignore this warning";


>Index: src/browser/BrowserWrapper.mm
>===================================================================

>-// IBAction from the safe browsing bar, sent from the "Get me out of here" button.
>+// Sent when the user chooses to leave a dangerous page via the "Get Me Out" button.

Using spaces in the button reference there seems a bit odd, since we have no button whose key or label is "Get Me Out".  Maybe use the whole key there?  Dunno.

Anyway, behavior is good, "Close Page" looks good as the label, and the nib looks fine (save our friend bug 509670); r=ardissone

Comment 12

[Stuart Morgan]

Comment on attachment 394325 [details] [diff] [review]
Patch, v2 (From updated source tree)

sr=smorgan, but I'm with Smokey on using getMeOutButton or similar in the comment.

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Checked in on CAMINO_2_0_BRANCH and cvs trunk, including the nib fix for bug 509670.  

I went with "Close Page" to address Stuart's/my comment, because the other comment appeared to be wanting to use the literal button name in quotes:

> // IBAction from the safe browsing bar, sent from the "Report Incorrect Site" button.

However, as it turns out, that's not exactly literal button name, so we may want to change that comment, too.  That comment is at least close enough that it shouldn't become ambiguous over time, though.  (Also, the "Report an error…" button name doesn't have a corresponding key in Localizable.strings, since it's nib-only, so that does argue for using the literal English button names.)