653146 - switch awesomescreen tabs immediately on tap, then load content

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Madhava Enros [:madhava]]

Right now, when you tap on some awesomescreen tabs (i.e. remote tabs or history), sometimes nothing happens immediately. The browser eventually brings up the selected tab, it seems, when it has loaded the contents.  Instead, we should switch tabs immediately, and then show some progress indication, if necessary.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Lets see if we can make some impact on this for FF6

Comment 2

[Madhava Enros [:madhava]]

Design-wise, let's visually switch to the tab and have our throbber displayed in the empty list area below.  Maybe centered, vertically and horizontally, in the area until we have anything to show?

Actually, when items are shown, how do they appear? One by one? All at once?

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

The patch will probably resolved bug 653146 too

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 532228 [details] [diff] [review]
Patch

>+/* awesomescreen panels ---------------------------------------------------- */
>+historylist > hbox,
>+remotetabslist > hbox {
>+  display: none;
>+}
>+
>+historylist[loading="true"] > hbox,
>+remotetabslist[loading="true"] > hbox {
>+  background-color: white;
>+  display: -moz-box;
>+}
>+
>+historylist[loading="true"] > richlistbox,
>+remotetabslist[loading="true"] > richlistbox {
>+  visibility: collapse;
>+}

Add a class to the XBL (hbox and richlistbox) so you can be more specific. The richlistboxes already have a class, so use it.

"history-throbber-box" and "remotetabs-throbber-box" ?

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

done!

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 532236 [details] [diff] [review]
Patch v0.2

passing to Wes - I just did a drive by :)

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #7)
> Comment on attachment 532236 [details] [diff] [review] [review]
> Patch v0.2
> 
> passing to Wes - I just did a drive by :)

oups that was my initial intention but my fingers has just type your name without my consent!

Comment 9

[Wesley Johnston (:wesj)]

Comment on attachment 532236 [details] [diff] [review]
Patch v0.2

>+      <xul:hbox class="history-throbber-box" flex="1" align="center" pack="center">
>+        <xul:image src="chrome://browser/skin/images/throbber.png" />
>+      </xul:hbox>

Do we need the image element, or can we just use background-image? If you leave the image in, source should be set in css using "list-style-image" on the box. Same in the other place

>+            let children = self._children;
>+            while (children.lastChild)
>+              children.removeChild(children.lastChild);

Seems cleaner to me to do it outside the setTimeout and not worry about hiding these in CSS, but I guess this should do the same... except faster? If so, neat!

>+            self.setAttribute("loading", "false");

Just remove the attribute.

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mozilla-central/rev/3da7cbddd83e

Comment 11

[Andreea Pod]

Verified fixed on build: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110520 Firefox/6.0a1 Fennec/6.0a1 
Device: LG Optimus 2X (Android 2.2)