451373 - code cleanup in BrowserUI.show

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Daniel Brooks [:db48x]]

I cleaned it up a bit, and figure that the cleanup can be checked in independently. This is mostly just to make it more easily maintainable; it doesn't make it easy for an extension author to use. It does make it easier for us to add to the list in the future.

Comment 1

[Daniel Brooks [:db48x]]

Attachment: WIP

Comment 2

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

Comment on attachment 334680 [details] [diff] [review]
WIP

I don't think we should go this route. Things are only going to get worse and the current (and WIP) approaches don't scale.

I have some ideas. I hope to get a patch soon.

Comment 3

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

"Things are only going to get worse" -> Fennec is getting more panels and sidebars

Comment 4

[Daniel Brooks [:db48x]]

Attachment: 451373-2.diff

In that vein, here's something simple we can do to make extensions possible.

Comment 5

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

Comment on attachment 340793 [details] [diff] [review]
451373-2.diff

I like this, but I just bitrotted your patch pretty good.

Also, I'd like to use "linkedpanel" instead of "panelid" as the attribute on the toolbarbuttons

Comment 6

[Daniel Brooks [:db48x]]

Attachment: 451373-3.diff

Comment 7

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

Comment on attachment 340811 [details] [diff] [review]
451373-3.diff

nice cleanup

Comment 8

[Daniel Brooks [:db48x]]

Comment on attachment 340811 [details] [diff] [review]
451373-3.diff

I pushed this the other day.

Comment 9

[Daniel Brooks [:db48x]]

The reason I didn't resolve the bug was that there were/are other improvements that could be made. I guess that is uncouth though.

Comment 10

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

we have reworked a lot of this code.  This is more of a dev item than a testcase to verify.