Last Comment Bug 678480 - First-run animation is busted and tab sidebar is visible at startup
: First-run animation is busted and tab sidebar is visible at startup
Status: VERIFIED FIXED
: regression
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Firefox 9
: ARM Android
: -- normal (vote)
: Firefox 9
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
Mentors:
: 680410 (view as bug list)
Depends on: 689928 691541
Blocks: 610834 625229
  Show dependency treegraph
 
Reported: 2011-08-12 06:15 PDT by Aaron Train [:aaronmt]
Modified: 2011-10-24 12:01 PDT (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (1.10 KB, patch)
2011-09-22 08:41 PDT, Mark Finkle (:mfinkle) (use needinfo?)
wjohnston2000: review+
Details | Diff | Review
patch 2 (3.24 KB, patch)
2011-09-22 16:39 PDT, Mark Finkle (:mfinkle) (use needinfo?)
wjohnston2000: review+
Details | Diff | Review

Description Aaron Train [:aaronmt] 2011-08-12 06:15:28 PDT
Mozilla/5.0 (Android; Linux armv7l; rv:8.0a1) Gecko/20110812 Firefox/8.0a1 Fennec/8.0a1

20110812030744
http://hg.mozilla.org/mozilla-central/rev/f262c389193e

STR: New Profile, Launch Nightly
Comment 2 Wesley Johnston (:wesj) 2011-08-12 13:10:20 PDT
Hmm... can't reproduce this with my locale builds, but I see it on nightly...
Comment 3 Aaron Train [:aaronmt] 2011-08-13 18:25:37 PDT
Something fixed this (something backed out?), now working on 08/13

Verified WFM
Mozilla/5.0 (Android; Linux armv7l; rv:8.0a1) Gecko/20110813 Firefox/8.0a1 Fennec/8.0a1
Comment 4 Aaron Train [:aaronmt] 2011-08-19 06:13:57 PDT
Hm, this is back again.

Build: Mozilla/5.0 (Android; Linux armv7l; rv:9.0a1) Gecko/20110819 Firefox/9.0a1 Fennec/9.0a1
Device: Samsung Galaxy SII,
Comment 5 Kevin Brosnan [:kbrosnan] 2011-08-22 09:54:13 PDT
*** Bug 680410 has been marked as a duplicate of this bug. ***
Comment 6 Matt Brubeck (:mbrubeck) 2011-08-29 15:47:07 PDT
This is very likely a regression from bug 610834, which landed, was backed out, and re-landed at the times this bug appeared, disappeared, and reappeared.
Comment 7 Matt Brubeck (:mbrubeck) 2011-09-06 11:54:04 PDT
I'm also seeing the tab bar exposed on startup on non-firstrun some of the time, which I believe is related to this bug.  On my HTC T-Mobile G2 (Android 2.3), I see this problem only when about:home is shown at startup.

Requesting tracking for Fx9.  If we can't fix this for Fx9, we should back out bug 610834.
Comment 8 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-09-21 15:11:38 PDT
Bug 686903 removed the fade in animation.
Comment 9 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-22 08:03:14 PDT
I think we should consider leaving bug 610834 in since it fixes a more general issue. We have code at startup that is very specific to the way the previous "resize" events were sent. I'd rather fix (or remove) the busted UI code.

The affected code seems to be the "hideSidebar" code in the resize handler. The code thinks the sidebars have been hidden, but they really have not.
Comment 10 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-22 08:41:06 PDT
Created attachment 561756 [details] [diff] [review]
patch

This is a silly patch that forces XUL to layout and setup the scrollbox. Once the scrollbox is setup, the call to hideSidebar seems to work fine.

With this patch, the sidebar is not shown at startup and the animated transitions work in about:home. Tested on Nexus One.
Comment 11 Wesley Johnston (:wesj) 2011-09-22 08:55:16 PDT
Comment on attachment 561756 [details] [diff] [review]
patch

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

Probably should add a comment. I'm checking if getPosition({}, {}) works, but have to rebuild. Doesn't seem like a big deal to just use this though.
Comment 12 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-22 16:39:20 PDT
Created attachment 561923 [details] [diff] [review]
patch 2

Last patch was still broken.

This patch uses the scroll position to verify something actually worked. It also updates the "x" property with the new scroll position so the next time we see the resize, we don't reset the scroll to zero.

I also tweaked the firstrun animation code in aboutHome. We use a timeout to start the animation so we don't conflict with the code to hide the sidebar. We also use the flipped logic in startDiscovery to check for the sidebars being opened. Sometimes "leftWidth" was 0.0555, so I just check to see if the sidebar is completely open instead. 

I tested:
* initially opening up to home page -> got animation, then sidebar was hidden
* subsequent opening -> sidebar hidden
* reset the ui discovery pref and opened about:home with the sidebar already visible -> no animation

I think that covers it
Comment 13 Wesley Johnston (:wesj) 2011-09-22 16:50:18 PDT
Comment on attachment 561923 [details] [diff] [review]
patch 2

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

Everything about this feels kinda like a hack and makes me nervous. Can you add some comments explaining what's going on in various places?
Comment 14 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-22 19:34:00 PDT
Added comments and found I could remove the setTimeout hack. Retested and pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cbc62f5e79e8
Comment 15 Ed Morley [:emorley] 2011-09-23 04:41:36 PDT
https://hg.mozilla.org/mozilla-central/rev/cbc62f5e79e8
Comment 16 Aaron Train [:aaronmt] 2011-09-25 06:51:26 PDT
Mozilla/5.0 (Android; Linux armv7l; rv:9.0a1) Gecko/20110925 Firefox/9.0a1 Fennec/9.0a1

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