1146735 - Change outer container in @layout/anchored_popup to FrameLayout

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

Lucas once told me FrameLayout is more performant than LinearLayout because it does away with a lot of measuring code in layout - the anchored popup outermost container, used to draw the shadow background, uses LinearLayout [1] even though it has one child (and will forever have one child, as far as I can tell).

It should be changed to a FrameLayout.

Chenxia, mentorable?

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/anchored_popup.xml?rev=534e729ad1d7#6

Comment 1

[Amit singh]

hello again its me i want to work on this bug. wait ill submit my patch

Comment 2

[Ravi Shankar [:waffles]]

Go ahead. If you have any questions, please use the needinfo box pointing to your mentor.

Comment 3

[Amit singh]

ok this bug is in firefox for android.I want the screen shot if possible what exactly happening.

Comment 4

[Chenxia Liu [:liuche]]

Hi Amit, do you have a working build of Firefox for Android? If not, you can get started here:
https://wiki.mozilla.org/Mobile/Fennec/Android#Preparing_your_build_environment

After that, you can make a patch and make sure it follows the patch guidelines. Basically, this bug is for replacing the LinearLayout with a FrameLayout at this point: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/anchored_popup.xml?rev=534e729ad1d7#6

Afterwards, the image should look the same as before, but should be more performant. You can upload screenshots of before/after, to make sure they haven't changed.

Comment 5

[Justin Inouye [:inouju]]

Attachment: anchored_popup change to FrameLayout

I'm not quite sure if I did the patch attachment correctly, but I talked to Chenxia on irc and said I could take this bug.

Comment 6

[Chenxia Liu [:liuche]]

Comment on attachment 8650759 [details] [diff] [review]
anchored_popup change to FrameLayout

Hi Justin, this patch is in fact not in the right format :)

but you can take a look here and see how you should make it.

https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Clearing the review flag for now.

Comment 7

[Justin Inouye [:inouju]]

Attachment: bug-1146735-fix

Sorry! I think this should be right now, but let me know if anything is missing.

Comment 8

[Chenxia Liu [:liuche]]

Comment on attachment 8651066 [details] [diff] [review]
bug-1146735-fix

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

Nice.

Comment 9

[Pulsebot]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bf111f5f87d7