Closed Bug 464146 Opened 11 years ago Closed 11 years ago

about:rights notification sometimes not shown with session restore

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 3.1b2

People

(Reporter: Dolske, Assigned: Dolske)

References

Details

(Keywords: verified1.9.0.5, verified1.9.1, Whiteboard: [in-litmus-bug-week])

Attachments

(1 file, 1 obsolete file)

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+
Attached patch Patch v.1 (WIP) (obsolete) — Splinter Review
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?
(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.
Attached patch Patch v.2Splinter Review
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.
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?
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+
Pushed: http://hg.mozilla.org/mozilla-central/rev/0cd41f599080
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1b2
needs testcase?
Flags: in-testsuite?
Flags: in-litmus?
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?
Checking in browser/components/nsBrowserGlue.js;
  new revision: 1.100; previous revision: 1.99
Flags: in-testsuite? → in-testsuite-
Keywords: fixed1.9.0.5
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.
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
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.