Closed Bug 676711 Opened 13 years ago Closed 11 years ago

tab thumbnail is sometimes outdated

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached image screenshot
See screenshot.

The android dev website was down and when it was finally up, the icon wasn't updated...
This happens because we do not keep updating the preview. We take a snapshot once the page initially loads, and that's it.
Summary: The preview icon in the left panel is sometimes outdated → tab thumbnail is sometimes outdated
Duping. Reopen if you mean something different
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → DUPLICATE
I didn't really meant to have the thumbnail being live only updated on page load. I don't think it's a good behavior to refresh a page, get another content but don't change the thumbnail.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Attached patch Patch v1 (obsolete) — Splinter Review
What about this?
Assignee: nobody → mounir
Status: REOPENED → ASSIGNED
Attachment #552776 - Flags: review?(mark.finkle)
Whiteboard: [needs review]
Comment on attachment 552776 [details] [diff] [review]
Patch v1

We should check HG blame for the reason why we added _drawThumb. It was not always there, so there must be a reason (bug) for why we added it.
(In reply to Mark Finkle (:mfinkle) from comment #5)
> Comment on attachment 552776 [details] [diff] [review]
> Patch v1
> 
> We should check HG blame for the reason why we added _drawThumb. It was not
> always there, so there must be a reason (bug) for why we added it.

If I understand it correctly, the idea was to draw the thumbnail after the first paint and after the page load. In other words, with my patch, it will be drawn twice. If this is sensitive, we could probably set drawThumb to true at startLoading. I believe the result would be the same.
Attached patch Patch v2Splinter Review
So I did have a deeper look and _drawThumb was here to be sure we draw the thumbnail when the first paint was done AND the page was loaded. We were trying to draw the thumbnail at the first paint but were canceling that if the page was still loading. At page load we were always updating the thumbnail. Both calls were making sure the thumbnail was updating in at least one situation.

My patch makes sure we call updateThumbnail() only when the loading is done and we makes us listening to the first paint if the first paint isn't done at that point.
Attachment #552776 - Attachment is obsolete: true
Attachment #552776 - Flags: review?(mark.finkle)
Attachment #560073 - Flags: review?(mark.finkle)
Comment on attachment 560073 [details] [diff] [review]
Patch v2

>   endLoading: function endLoading() {

>+    if (!this._delayedThumbnail) {
>+      if (this._firstPaint) {
>+        this.updateThumbnail();
>+      } else if (!this._delayedThumbnail) {

Can't this just be an "else" ? We are already in | if (!this._delayedThumbnail) |

r+ with the "else" change, if it's OK
Attachment #560073 - Flags: review?(mark.finkle) → review+
Is there a way to add a test for this situation? If so, can you make one?
(In reply to Mark Finkle (:mfinkle) from comment #10)
> We have some simple thumbnail tests here:
> http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/tests/
> browser_thumbnails.js

Unfortunately, this test file doesn't check the thumbnail content but the position and size. I wonder how I could test the content and I believe I should do that later. It will require me to do testing on my phone which I still haven't done yet.
Flags: in-testsuite?
Whiteboard: [needs review] → [needs landing]
Whiteboard: [needs landing] → [needs tests][inbound]
Attachment #560073 - Flags: checkin+
Whiteboard: [needs tests][inbound] → [needs tests fixes]
Attachment #560073 - Flags: checkin+ → checkin-
Blocks: 627354
I was able to reproduce this issue by following these steps:
1. Open Fennec
2. Open a new blank tab
3. Bookmark the blank page opened at step 2
4. In the same tab, browse to any webpage (ex: www.mozilla.com) and wait until the page is fully loaded
5. Tap on URL Bar > Bookmarks tab
6. Tap on about:blank page 
7. Check the tab thumbnail if it's updated

Actual result:
After step 7, there will be the same thumbnail at it was after step 4.

Notes:
- after step 7, if about:firefox will be opened in the same tab, actually it will open in a new tab. For the one opened at step 2, about:firefox will be auto-filled in the URL Bar.
- this issue occurs on the latest Aurora and Nightly builds.

--
Build ID: Mozilla/5.0 (Android;Linux armv7l;rv:10.0a1)Gecko/20111009
Firefox/10.0a1 Fennec/10.0a1
Device: Samsung Galaxy S
OS: Android 2.2
Time has passed and this patch no longer makes sense (it was for the XUL product).
Status: ASSIGNED → RESOLVED
Closed: 13 years ago11 years ago
Resolution: --- → INVALID
Whiteboard: [needs tests fixes]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: