1150621 - Set TabsPanel.TabsPanelToolbar attributes in xml, rather than code

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Currently, some attributes seem to be defined dynamically:

https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tabs/TabsPanel.java?rev=83ffa6446310#355

Move these values to layout/tabs_panel_default.xml and make sure nothing changes in the TabsPanel (click the tab count button). Test on tablet too if you can!

To start, set up a build environment - you can see the instructions here: https://wiki.mozilla.org/Mobile/Fennec/Android

Then, you'll need to create a patch to upload - see
https://wiki.mozilla.org/Mobile/Fennec/Android#Creating_commits_and_submitting_patches

If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "mcomella" and you can find me in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC

Thanks and happy coding! ^_^

Comment 1

[u535116]

Anybody taking this? I'd like to work on this one. Thanks!

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Hey, Matt - welcome aboard! :) Go ahead and get started - whenever you upload your first patch, I'll assign you to the bug.

Comment 3

[u535116]

Attachment: Set TabsPanel.TabsPanelToolbar attributes in xml, rather than code

I also removed setLayoutParams in TabsPanel, since it seemed as unnecessary as the call in TabsPanelToolbar

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0dc6889c806f

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8589590 [details] [diff] [review]
Set TabsPanel.TabsPanelToolbar attributes in xml, rather than code

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

Nice!

I made a push to our try test servers (see above).

Once it goes green, feel free to add the checkin-needed keyword [1]. Let me know if you need help reading the results. Note that all patches that use "checkin-needed" must also have an associated green try run.

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

If you'd like to get working on another bug, perhaps bug 952290 or bug 1105271 strikes your fancy? If not, let me know and I can dig up some more bugs.

Comment 7

[u535116]

Thanks so much for the help :mcomella, and I'm taking a look at those other bugs now!

Comment 8

[Carsten Book [:Tomcat]]

Hi,

sorry this didn't apply:

renamed 1150621 -> tabspaneltoolbar-attributes.patch
applying tabspaneltoolbar-attributes.patch
unable to find 'mobile/android/base/resources/layout-large-land-v11/tabs_panel_sidebar.xml' for patching
2 out of 2 hunks FAILED -- saving rejects to file mobile/android/base/resources/layout-large-land-v11/tabs_panel_sidebar.xml.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh tabspaneltoolbar-attributes.patch

could you take a look, thanks!

Comment 9

[u535116]

Attachment: Set TabsPanel.TabsPanelToolbar attributes in xml, rather than code

Comment 10

[u535116]

Weird, I don't know why that file only exists locally. Submitted a new patch. Thanks!

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8591675 [details] [diff] [review]
Set TabsPanel.TabsPanelToolbar attributes in xml, rather than code

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

That file was removed in a previous changeset (bug 1106935, I believe, which just landed) - thanks again, Matt!

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/fx-team/rev/8bcd9d38bb20

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8bcd9d38bb20