No "Undo closed tab" after closing a restored tab

VERIFIED FIXED in Firefox 9

Status

Fennec Graveyard
General
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Andreea Pod, Assigned: lucasr)

Tracking

Trunk
Firefox 9
ARM
Android

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Mozilla /5.0 (Android;Linux armv7l;rv:9.0a1) Gecko/20110824 Firefox/9.0a1 Fennec/9.0a1

Mozilla /5.0 (Android;Linux armv7l;rv:7.0) Gecko/20110824 Firefox/7.0 Fennec/7.0

Device: LG Optimus 2X (Android 2.2)

Steps to reproduce:
1. Open a few tabs
2. Kill the application using a task killer
3. Open Fennec and see tabs are restored
4. Close tabs that are not in focus and are not loaded

Expected result:
Tabs should go to "undo closed tab"

Actual result:
Tabs are disappearing and you are not able to undo them, please see the video: http://www.youtube.com/watch?v=pwJL26t6pA8&feature=channel_video_title
(Assignee)

Comment 1

6 years ago
Taking.
Assignee: nobody → lucasr.at.mozilla
(Assignee)

Comment 2

6 years ago
Created attachment 560283 [details] [diff] [review]
Remove "empty" attribute from the thumbnails of restored tabs
Attachment #560283 - Flags: review?(mark.finkle)
Attachment #560283 - Flags: review?(mark.finkle) → review+
Keywords: checkin-needed

Comment 3

6 years ago
Will push to inbound once try goes green:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=179780dcf805
Status: NEW → ASSIGNED
Keywords: checkin-needed

Comment 4

6 years ago
Previous try run failed to actually show any results and build api says that rev is 404 (no idea why), so pushed again:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=19373e00fd85

Comment 5

6 years ago
Ok, so that try push failed to insert into the schedulerdb as well, so I've just pushed this to inbound to save spending any longer on this.

https://hg.mozilla.org/integration/mozilla-inbound/rev/5131fb5635b5
Target Milestone: --- → Firefox 9

Comment 6

6 years ago
https://hg.mozilla.org/mozilla-central/rev/5131fb5635b5
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110921
Firefox/9.0a1 Fennec/9.0a1
Device: HTC Desire
OS: Android 2.2

I was able to reproduce this issue using the initial steps.
It looks like you cannot Undo a tab closed before the page is fully loaded.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 8

6 years ago
Could not reproduce this. Catalin, what do you mean by "looks like you cannot Undo a tab closed before the page is fully loaded"? Closing the restored tab before the initial page is fully loaded?
I was able to reproduce this on the latest nightly.
This are the steps:
1. Open Fennec
2. Load two heavy content pages in two new tabs (I used: www.ziare.com and www.bbc.co.uk)
3. "Force stop" nightly from Settings | App.
4. Start nightly
5. Right after nightly starts and tabs start loading, close one or both tabs. Don't wait till tabs are updated with the thumbnail picture

Actual:
"Undo closed tab" button doesn't show up.
(Assignee)

Comment 10

6 years ago
If you remove the session-restored tab before it loads its thumbnail, we still consider the tab as "empty" and no undo operation will be offered. I can just set all session-restored tabs as non-empty before the thumbnail is even loaded to avoid this problem. Patch coming.
(Assignee)

Comment 11

6 years ago
Created attachment 562754 [details] [diff] [review]
Immediately remove "empty" attribute from session-restored tabs
Attachment #560283 - Attachment is obsolete: true
Attachment #562754 - Flags: review?(mark.finkle)
Comment on attachment 562754 [details] [diff] [review]
Immediately remove "empty" attribute from session-restored tabs

Looks OK. I looked in MXR for all occurrences of "empty" in /mobile and found that tabs.xml uses it. But this change seems to not break the usage.

I also found "empty" used in a test:

http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/tests/browser_tabs.js#130

This change should not break this use either.
Attachment #562754 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 13

6 years ago
Yeah, this is a very specific change that only applies to session restored tabs.
Keywords: checkin-needed
(Assignee)

Comment 14

6 years ago
Landed: http://hg.mozilla.org/integration/mozilla-inbound/rev/f3022823eb87

Mark, I guess this should land in aurora too?
Keywords: checkin-needed
(In reply to Lucas Rocha (:lucasr) from comment #14)
> Landed: http://hg.mozilla.org/integration/mozilla-inbound/rev/f3022823eb87
> 
> Mark, I guess this should land in aurora too?

Not sure this is critical enough to push for Aurora. Do you think this is common enough and/or causes a bad enough problem that we want to get it into Aurora?

I assume this is a regression from Fx8, right?

Comment 16

6 years ago
https://hg.mozilla.org/mozilla-central/rev/f3022823eb87
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 17

6 years ago
(In reply to Mark Finkle (:mfinkle) from comment #15)
> (In reply to Lucas Rocha (:lucasr) from comment #14)
> > Landed: http://hg.mozilla.org/integration/mozilla-inbound/rev/f3022823eb87
> > 
> > Mark, I guess this should land in aurora too?
> 
> Not sure this is critical enough to push for Aurora. Do you think this is
> common enough and/or causes a bad enough problem that we want to get it into
> Aurora?
> 
> I assume this is a regression from Fx8, right?

I fixed this bug in Fx9 but it got reopened and fixed after the merge. This is why I was wondering whether this is Aurora material or not. I don't think it's critical to push for Aurora.

Comment 18

6 years ago
Retested bug with:

Build ID: Mozilla/5.0 (Android;Linux armv7l;rv:10.0a1)Gecko/20110930
Firefox/10.0a1 Fennec/10.0a1
Device: Motorola DROID 2
OS: Android 2.3

and

BUILD ID: Mozilla/5.0 (Android;Linux armv7l;rv:9.0a2)Gecko/20110930
Firefox/9.0a2 Fennec/9.0a2

Bug is no longer reproducible. 

"Undo Closed tab" is available after closing a restored tab.

Verifying bug.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.