Last Comment Bug 718465 - After opening a new tab, old tab is briefly seen
: After opening a new tab, old tab is briefly seen
Status: VERIFIED FIXED
startup-ux, tabs-ux, ui-responsiveness
: polish
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
: P3 normal (vote)
: Firefox 19
Assigned To: Brian Nicholson (:bnicholson)
:
:
Mentors:
Depends on: 720494 797075 803374
Blocks: 721008 740335
  Show dependency treegraph
 
Reported: 2012-01-16 10:08 PST by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2016-07-29 14:21 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
affected
verified
-
11+


Attachments
Part 1: Only send SELECTED tab event if the tab has changed (1.02 KB, patch)
2012-10-17 15:23 PDT, Brian Nicholson (:bnicholson)
mark.finkle: review+
Details | Diff | Splinter Review
Part 2: Clear background color when selected tab changes (7.24 KB, patch)
2012-10-17 15:59 PDT, Brian Nicholson (:bnicholson)
bugmail: review+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2012-01-16 10:08:08 PST
Steps to reproduce:
- Open a new tab, by tapping on the "+"-sign in the tab list popup.
- Choose one of the autocomplete entries.

Expected result:
- The new tab is immediately shown with the chosen url that gets loaded.

Acctual result:
- For a brief moment, the old tab is shown, then the new tab gets opened with the chosen url that gets loaded.
Comment 1 :Margaret Leibovic 2012-01-18 16:18:43 PST
It looks like this is because we're waiting for the Tab:Added message to get back to us from Gecko before calling handleSelectTab, which changes the tab in the Java UI. I guess we need to have information about the tab from Gecko to update the UI properly, but maybe we can cheat and make a stub new tab that gets populated after the fact.

The same thing is true of switching tabs - we do a message passing round trip to Gecko and back before updating the Java UI.
Comment 2 Doug Turner (:dougt) 2012-01-23 20:01:44 PST
margaret, over to you since you know more about this than most.  if you think there is a better person to own this, please reassign.
Comment 3 :Margaret Leibovic 2012-01-24 08:49:26 PST
I was looking into doing the same thing to the Tab:Add(ed) messages that I did with Tab:Close(ed) and Tab:Select(ed), but it's more complicated since we're actually using parameters we get back from Gecko when we open the new tab. However, we don't need to wait on Gecko to know that we need a new tab in the UI, and we know the new URL, so we could at least update BrowserToolbar immediately and continue to update it in with more data as we get it.
Comment 4 Ian Barlow (:ibarlow) 2012-04-10 13:29:03 PDT
Soft blocker nom?
Comment 5 Brian Nicholson (:bnicholson) 2012-10-11 10:41:57 PDT
Looks like bug 797075 was sufficient to fix this problem.
Comment 6 Cristian Nicolae (:xti) 2012-10-15 05:09:30 PDT
I can reproduce this issue on the latest Nightly even if bug 797075 has already landed. Which bug should be reopened?

--
Firefox 19.0a1 (2012-10-15)
Device: Galaxy Note
OS: Android 4.0.4
Comment 7 Brian Nicholson (:bnicholson) 2012-10-15 11:02:13 PDT
(In reply to Cristian Nicolae (:xti) from comment #6)
> I can reproduce this issue on the latest Nightly even if bug 797075 has
> already landed. Which bug should be reopened?
> 
> --
> Firefox 19.0a1 (2012-10-15)
> Device: Galaxy Note
> OS: Android 4.0.4

Depending on what is meant by seeing the "old tab", there are actually two parts to this bug:
1) the URL bar/throbber is not updated immediately, and
2) the actual drawn page is not updated immediately

Bug 797075 should have fixed the first issue. The second issue (which wouldn't have been fixed) is more complicated because the LayerView won't be updated until Gecko draws to it - so stubbing tabs can't help here. Is this the problem you're talking about?
Comment 8 Cristian Nicolae (:xti) 2012-10-15 23:19:30 PDT
(In reply to Brian Nicholson (:bnicholson) from comment #7)
> (In reply to Cristian Nicolae (:xti) from comment #6)
> 
> Depending on what is meant by seeing the "old tab", there are actually two
> parts to this bug:
> 1) the URL bar/throbber is not updated immediately, and
> 2) the actual drawn page is not updated immediately
> 
> Bug 797075 should have fixed the first issue. The second issue (which
> wouldn't have been fixed) is more complicated because the LayerView won't be
> updated until Gecko draws to it - so stubbing tabs can't help here. Is this
> the problem you're talking about?

It seems that only the 2nd part of this issue is still reproducible. The first one is fixed indeed.
Comment 9 Brian Nicholson (:bnicholson) 2012-10-15 23:35:27 PDT
Fixing this part will be more complicated. We'd likely have to resort to setting a background color on the LayerView, then clearing the background color after a draw for new tabs (similar to bug 799617).
Comment 10 Cristian Nicolae (:xti) 2012-10-15 23:43:53 PDT
(In reply to Brian Nicholson (:bnicholson) from comment #9)
> Fixing this part will be more complicated. We'd likely have to resort to
> setting a background color on the LayerView, then clearing the background
> color after a draw for new tabs (similar to bug 799617).

Is it possible to draw a temporary background with the Firefox logo, like it happens on about:home for those websites which don't have a thumbnail saved?
Comment 11 Brian Nicholson (:bnicholson) 2012-10-16 00:23:43 PDT
I guess we'd need UX to weigh in here. I think a white background might make sense because it makes the page feel like it's loading immediately. Also, on desktop Firefox, opening a URL in a new tab (ctrl+shift clicking on a link) shows all white before the page is drawn.
Comment 12 Cristian Nicolae (:xti) 2012-10-16 00:27:47 PDT
You're right. I was thinking about that logo just to make a nice difference between desktop and Firefox for Android, but the sensation would be that even if the page is loading, nothing happens.
Comment 13 Brian Nicholson (:bnicholson) 2012-10-17 15:23:06 PDT
Created attachment 672548 [details] [diff] [review]
Part 1: Only send SELECTED tab event if the tab has changed

Without this change, selecting the tab that is already selected causes the LayerView color to change without getting cleared (since a draw isn't retriggered).
Comment 14 Brian Nicholson (:bnicholson) 2012-10-17 15:59:41 PDT
Created attachment 672569 [details] [diff] [review]
Part 2: Clear background color when selected tab changes

This builds upon bug 799617 to clear the LayerView every time a tab is selected.

Since there's some delay between selecting the tab and drawing its contents, this causes just the background to be shown for a short period when a tab is selected. This is better to some extent because the view is instantly updated when the user switches tabs, but it also feels weird to show another state during the process. That is: before, it showed the old page with a bit of delay after switching tabs. Now, it shows the old page, then the background immediately after switching tabs, then the tab content.
Comment 15 Kartikaya Gupta (email:kats@mozilla.com) 2012-10-17 20:29:55 PDT
Comment on attachment 672569 [details] [diff] [review]
Part 2: Clear background color when selected tab changes

Review of attachment 672569 [details] [diff] [review]:
-----------------------------------------------------------------

The only hiccup I see here is that LayerRenderer.onTabChanged gets run on the UI? thread whereas the other pieces of code that touch the paint state run on the compositor thread. I can't think of any interleaving that would actually cause problems though. Maybe just add a comment in onTabChanged so if we end up with strange behaviour later we can check to see if that's causing it.
Comment 18 Cristian Nicolae (:xti) 2013-01-30 06:36:25 PST
This issue is not reproducible on the latest Nightly. Closing bug as verified fixed on:

Firefox for Android
Version: 21.0a1 (2013-01-29)
Device: Galaxy R
OS: Android 2.3.4

Note You need to log in before you can comment on or make changes to this bug.