509525 - Floating the toolbar is really slow.

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Doug Turner (:dougt)]

Removing tryFloatToolbar and tryUnfloatToolbar from browser.js dramatically speed up the pan.  there is a claim that roc has a patch that fixes this performance problem.

Comment 1

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

those methods can also call "appendChild" to move the toolbar around, which can be slow.

Comment 2

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

Attachment: WIP-0.1

The original goal of this bug is to remove the appendChild mechanism living in tryFloatToolbar/tryUnfloatToolbar.
Basically it used a 'position: fixed' urlbar.

Comment 3

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

Comment on attachment 394002 [details] [diff] [review]
WIP-0.1

The patch has some problems, it just a proposal to see if I need to continue in this direction.

Comment 4

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

Attachment: WIP-2

Ok, I don't use any top/left hack here.
I Just want stuart feedback on that.

Comment 5

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

Attachment: WIP-3

A little explanation.

When the sidebars are visible this patch turns the urlbar into a fixed element positioned at 0,0. When they are invisible it return to the normal element flow.

The urlbar is container into a box that have the height of the it, so when we position: fixed the urlbar the content browser don't move to the top.

It's my last idea, not sure it is good.

Comment 6

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

Attachment: Patch v0.1

This is not as fast as we want but this has the advantage to remove the appendChild calls.

Comment 7

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

Attachment: Patch v0.2

Finally, I need to add one more box to the hierarchy, otherwise I have a size issue with urlbar-container (not toolbar-container) because the 'position: fixed' rule turned the display of the toolbar-main element into 'block', and in order to work flex need the parent node to have a display of '-moz-box' or '-moz-box-inline', ... so the urlbar-container won't fit the available space but use the size of it's content instead.

Comment 8

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

Attachment: Remove the move by a pixel bug

The move by a pixel bug is related to the display type of the sidebar, it differs depending on the container.

This version correct that.

Theere is also another little pixel bug but I've filled another bug for this one : bug 510488

Comment 9

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

Attachment: Patch v0.2

Works better with zoom.

Comment 10

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

Attachment: Patch v0.4

Sorry, for the spam... I've forget to size the height of the container in the previous patch.

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

how can I verify this bug?

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

verified with 20090827 winmo nightly trunk by sliding the side bars in and out and the url bar becomes floating.  Should be instantaneous and it is!

Comment 13

[Dão Gottwald [:dao]]

bugspam