Go button does not appear in location bar immediately after opening Private Browsing

ASSIGNED
Assigned to

Status

()

Firefox
Private Browsing
--
trivial
ASSIGNED
7 years ago
7 years ago

People

(Reporter: Mathnerd314, Assigned: Mathnerd314)

Tracking

({uiwanted})

Trunk
uiwanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6

This appears to be from the misapplication in bug 471512 of the patch in bug 455553. The source has now moved past both, leaving a vestigial "valid = (uri.spec != "about:blank")" in browser/base/content/browser.js. This should instead use gInitialPages, so that pages such as Private Browsing and Session Restore show the Go button in the location bar.

Reproducible: Always

Steps to Reproduce:
1. Create a fresh profile.
2. Enter private browsing mode.
Actual Results:  
Go button (green arrow) is missing from the location bar.

Expected Results:  
Go button appears normally, as it does for blank tabs or after deleting the URL in a location bar.

If this is fixed, extensions such as Auto/Speed/Fast Dial should be able to modify gInitialPages to blank their URL instead of being forced to overwrite URLBarSetURL (investigation of which led me to this bug).
(Assignee)

Comment 1

7 years ago
Created attachment 458143 [details] [diff] [review]
patch

A (very small) patch to fix this bug.
(Assignee)

Comment 2

7 years ago
Created attachment 458182 [details] [diff] [review]
patch with testcase

Apparently testing is required for all patches; this should do.
Attachment #458143 - Attachment is obsolete: true
Attachment #458182 - Flags: review?(ehsan)

Comment 3

7 years ago
Comment on attachment 458182 [details] [diff] [review]
patch with testcase

The patch looks good to me, but I'd prefer Gavin to take a look at it as well.
Attachment #458182 - Flags: review?(gavin.sharp)
Attachment #458182 - Flags: review?(ehsan)
Attachment #458182 - Flags: review+

Updated

7 years ago
Assignee: nobody → mathnerd314...gph+mozilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Version: unspecified → Trunk
(Assignee)

Comment 4

7 years ago
Created attachment 458248 [details] [diff] [review]
proper patch with testcase

Stupid me, I flipped the condition. Maybe I can build trunk and actually test the patch tomorrow.
Attachment #458182 - Attachment is obsolete: true
Attachment #458248 - Flags: review?(gavin.sharp)
Attachment #458182 - Flags: review?(gavin.sharp)
(Assignee)

Comment 5

7 years ago
Indeed, it works. (apologies if this counts as bugspam)
The pageproxystate attribute wasn't originally intended to just control the state of the Go button - that's a relatively recent addition. IIRC its original purpose was to style the favicon differently based on whether the value in the URL bar was "edited" or not (i.e. whether it actually reflected the loaded page).

Given that, I'm not sure what the reasoning is behind this bug, really (or why the current check exists for about:blank specifically). What benefit is there to the go button being present for "initial pages"? I suppose that it may make sense to avoid the no-go-button/go-button transition when you start typing a URL into an empty URL bar... Is that the only justification for this?

Comment 7

7 years ago
(In reply to comment #6)
> Given that, I'm not sure what the reasoning is behind this bug, really (or why
> the current check exists for about:blank specifically). What benefit is there
> to the go button being present for "initial pages"? I suppose that it may make
> sense to avoid the no-go-button/go-button transition when you start typing a
> URL into an empty URL bar... Is that the only justification for this?

Telling people that the location bar space can be used to type stuff into and press Go?
We did in fact hide the go button even for about:blank as long as you hadn't typed anything. This changed in bug 547224 when the "isempty" attribute was removed.
Comment on attachment 458248 [details] [diff] [review]
proper patch with testcase

This will fade the favicon, which we don't want for "initial pages" in general.
Attachment #458248 - Flags: review?(gavin.sharp) → review-
(In reply to comment #8)
> We did in fact hide the go button even for about:blank as long as you hadn't
> typed anything. This changed in bug 547224 when the "isempty" attribute was
> removed.

We could undo that change by using [value=""], I guess?
No, the value attribute doesn't update automatically.

Anyway, we would probably restore the pre-bug 547224 behavior if we could, so I think this bug can be WONTFIXed.

Comment 12

7 years ago
(In reply to comment #11)
> Anyway, we would probably restore the pre-bug 547224 behavior if we could, so I
> think this bug can be WONTFIXed.

Is there a bug on file for that?
(In reply to comment #12)
> (In reply to comment #11)
> > Anyway, we would probably restore the pre-bug 547224 behavior if we could, so I
> > think this bug can be WONTFIXed.
> 
> Is there a bug on file for that?

Not that I know of.
(Assignee)

Updated

7 years ago
Keywords: uiwanted
You need to log in before you can comment on or make changes to this bug.