about:rights notification sometimes not shown with session restore

VERIFIED FIXED in Firefox 3.1b2

Status

()

VERIFIED FIXED
10 years ago
8 years ago

People

(Reporter: Dolske, Assigned: Dolske)

Tracking

({verified1.9.0.5, verified1.9.1})

Trunk
Firefox 3.1b2
verified1.9.0.5, verified1.9.1
Points:
---
Bug Flags:
blocking-firefox3.5 +
in-testsuite -
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [in-litmus-bug-week])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
Normally the about:rights notification is shown the first time someone installs the browser, but it can also happen in an existing profile if (1) the user never saw the notification before (ala Linux on 3.0.x), or (2) if we should in the future bump the about:rights version number to show the notification again.

If the notification bar is shown after session restore has restored tabs, sometimes the notification will quickly disappear. This is caused by the browser seeing a tab change location from about:blank to http://whatever.com, at which point we automatically remove the notification bar.

Steps to Reproduce:
1) In about:config, set browser.rights.override to false [this forces display of the notification]
2) Set browser to "show tabs and windows from last time"
3) Quit the browser with 2 tabs open, and the second tab selected.
4) Relaunch the browser.

Note that in step 3, leaving the first tab selected will not trigger the problem.
Flags: blocking1.9.0.6?
Flags: blocking-firefox3.1?
Flags: blocking-firefox3.1? → blocking-firefox3.1+
(Assignee)

Comment 1

10 years ago
Created attachment 348655 [details] [diff] [review]
Patch v.1 (WIP)

Ugh. This should be easy, but it's hard... Figuring out if the tab is going to change location seems tricky. The current location is almost always about:blank (but not always), but only session restore seems to trigger this bug. I haven't been able to reproduce it on firstrun for a profile, or with "Show my homepage" set for startup.

After some experimentation, this patch seems to be a way to figure out if session restore set history to load, but now the notification bar sometimes persists one page load too long (eg, siteA.com is restored, navigating to siteB.netshould remove the notification bar but doesn't).

So, I'm not sure determine the current state of loading tabs, such that the notification bar doesn't immediately disappear during startup but does disappear on the first navigation. :(
Maybe we should just set its persistence to "3" or something? Having it hang around for longer than necessary isn't that much of a problem, right?
(Assignee)

Comment 3

10 years ago
(In reply to comment #2)
> Maybe we should just set its persistence to "3" or something? Having it hang
> around for longer than necessary isn't that much of a problem, right?

Yeah. Well, maybe. I keep flipping back and forth...

On the one hand, it stays consistent with other notification bars and avoids being an annoying presence.

On the other hand, other notification bars disappear on page-change because they're basically page-specific (and so persistence doesn't make much sense). I guess there's also be something to be said for it being more discoverable if it sticks around for a bit, otherwise you might not notice it until it slides away.
You can always close it manually, so I don't think it not disappearing on its own is much of an annoyance.
(Assignee)

Comment 5

10 years ago
Created attachment 348708 [details] [diff] [review]
Patch v.2

I guess we can always change it later!
Attachment #348655 - Attachment is obsolete: true
Attachment #348708 - Flags: ui-review?(beltzner)
Attachment #348708 - Flags: review?(gavin.sharp)
Comment on attachment 348708 [details] [diff] [review]
Patch v.2

Add a comment explaining that the 3 is just an arbitrary number?
Attachment #348708 - Flags: review?(gavin.sharp) → review+
Attachment #348708 - Flags: ui-review?(beltzner) → ui-review+
Comment on attachment 348708 [details] [diff] [review]
Patch v.2

Makes sense to me, and better than it was.
(Assignee)

Comment 8

10 years ago
Comment on attachment 348708 [details] [diff] [review]
Patch v.2

I guess this should target FF3.0.5... When Linux users get updated to that version, they'll generally be shown the about:rights bar (since most have never accepted a Firefox EULA, as I understand it), and without the patch the bar might disappear before it's possible to read it. Patch is small, safe, and won't affect anything else.
Attachment #348708 - Flags: approval1.9.1b2?
Attachment #348708 - Flags: approval1.9.0.5?
(Assignee)

Updated

10 years ago
Flags: blocking1.9.0.6? → blocking1.9.0.5?
Comment on attachment 348708 [details] [diff] [review]
Patch v.2

a191b2=beltzner
Attachment #348708 - Flags: approval1.9.1b2? → approval1.9.1b2+
(Assignee)

Comment 10

10 years ago
Pushed: http://hg.mozilla.org/mozilla-central/rev/0cd41f599080
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1b2
needs testcase?
Flags: in-testsuite?
Flags: in-litmus?
(Assignee)

Comment 12

10 years ago
Can't automate this, because it's timing sensitive. See comment 0 for STR.
Attachment #348708 - Flags: approval1.9.0.5? → approval1.9.0.5+
Comment on attachment 348708 [details] [diff] [review]
Patch v.2

Talked this over with Justin and he's going to provide updated STR for QA.

Approved for 1.9.0.5. a=ss
Flags: blocking1.9.0.5?
(Assignee)

Comment 14

10 years ago
Checking in browser/components/nsBrowserGlue.js;
  new revision: 1.100; previous revision: 1.99
Flags: in-testsuite? → in-testsuite-
Keywords: fixed1.9.0.5
(Assignee)

Comment 15

10 years ago
Actually, no updated STR needed...

The slight confusion was because we realized that with bug 462254's attachment 346348 [details] [diff] [review], existing profiles upgraded to FF3.0.5 won't be shown about:rights anyway. But the STR in comment 0 sets the override pref, which unconditionally forces display of the about:rights notification on startup, and avoids the problem.
Verified fixed in 1.9.0.5 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5pre) Gecko/2008120105 GranParadiso/3.0.5pre.
Keywords: fixed1.9.0.5 → verified1.9.0.5
Keywords: fixed1.9.1
verified FIXED on builds:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090526 Minefield/3.6a1pre ID:20090526031623

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090526 Shiretoko/3.5pre ID:20090526031155
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
Litmus testcase added:
https://litmus.mozilla.org/show_test.cgi?id=12790
Flags: in-litmus? → in-litmus+
Whiteboard: [in-litmus-bug-week]
You need to log in before you can comment on or make changes to this bug.