Closed Bug 1138723 Opened 5 years ago Closed 5 years ago

Tablet does not inherit Material theme

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed
fennec 38+ ---

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(2 files, 1 obsolete file)

I believe values-large-v11/themes.xml overrides values-v21/themes.xml.
tracking-fennec: --- → ?
/r/4549 - Bug 1138723 - Move arrowPopupWidth to dimens (from theme attr). r=liuche
/r/4551 - Bug 1138723 - Remove GeckoBase theme in values-large-v11/ to inherit from Material on tablet. r=liuche

Pull down these commits:

hg pull review -r 2cd05437f22d7d2457da819043d9e8a8b1faf4fb
Attachment #8572031 - Flags: review?(liuche)
https://reviewboard.mozilla.org/r/4549/#review3771

::: mobile/android/base/resources/layout/arrow_popup.xml
(Diff revision 1)
> -    <ScrollView android:layout_width="?attr/arrowPopupWidth"
> +    <ScrollView android:layout_width="@dimen/arrow_popup_container_width"

This will actually be obsoleted by bug 1136469 which removes the arrow popup (and associated width) - you could save me the trouble of rebasing :P

Looks good though.
https://reviewboard.mozilla.org/r/4551/#review3773

Ship It!

::: mobile/android/base/resources/values-large-v11/themes.xml
(Diff revision 1)
> -        <item name="android:windowActionBar">false</item>

What is windowActionBar for? I see it in all the v11 resources, so make sure you don't need it for the v21 GeckoBase.
Comment on attachment 8572031 [details]
MozReview Request: bz://1138723/mcomella

https://reviewboard.mozilla.org/r/4547/#review3775
Attachment #8572031 - Flags: review?(liuche)
(In reply to Chenxia Liu [:liuche] from comment #3)
> What is windowActionBar for? I see it in all the v11 resources, so make sure
> you don't need it for the v21 GeckoBase.

Good call - filed bug 1139248.
https://reviewboard.mozilla.org/r/4551/#review3785

> What is windowActionBar for? I see it in all the v11 resources, so make sure you don't need it for the v21 GeckoBase.

Good call - filed bug 1139248.
Is this an r+? Sorry, it's a bit ambiguous.
Flags: needinfo?(liuche)
Comment on attachment 8572031 [details]
MozReview Request: bz://1138723/mcomella

<finkle.gif>
Flags: needinfo?(liuche)
Attachment #8572031 - Flags: review+
Comment on attachment 8572031 [details]
MozReview Request: bz://1138723/mcomella

Approval Request Comment
[Feature/regressing bug #]: bug 1097337

[User impact if declined]:
  Users on tablet do not get android L features

[Describe test coverage new/current, TreeHerder]: None

[Risks and why]:
  We move a value that was specified with an Android attribute (match_parent) to our dimensions (pixel values) via a custom constant (also added in this patch). This is the first time we do such a thing and I'm a little suspicious of it because it's out of the norm, however, StackOverflow says it works and it appears to work in testing. Specifically, this value is used to determine the width of the doorhanger so in the worst case the width of the doorhanger is incorrect. Also, we may incorrectly change the style of tablet, but this is unlikely and low risk. 

[String/UUID change made/needed]: None
Attachment #8572031 - Flags: approval-mozilla-aurora?
tracking-fennec: ? → 38+
https://hg.mozilla.org/mozilla-central/rev/75204e00b8cd
https://hg.mozilla.org/mozilla-central/rev/ae4c0736cb3c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Comment on attachment 8572031 [details]
MozReview Request: bz://1138723/mcomella

getting this into aurora so it gets to our beta audience for wider testing sooner.
Attachment #8572031 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 1146809
Depends on: 1140359
Attachment #8572031 - Attachment is obsolete: true
Attachment #8619641 - Flags: review+
Attachment #8619642 - Flags: review+
You need to log in before you can comment on or make changes to this bug.