Closed Bug 1097318 Opened 5 years ago Closed 5 years ago

Fix talos regression when new tablet UI is enabled

Categories

(Firefox for Android :: General, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 37
Tracking Status
firefox36 --- verified
firefox37 --- verified
firefox38 --- verified

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

Attachments

(1 file)

For some reason, when the chrome area is taller than 104dp, testCheck2 misbehaves and reports a massive regression. In bug 1097316, I'm keeping the chrome area exactly at 104dp as a temporary workaround. We need to figure out why the bigger chrome area causes regressions in testCheck2.
Priority: -- → P1
Comment on attachment 8524629 [details] [diff] [review]
Restore tab strip to its original height (r=mfinkle)

From IRC discussion with snorp: Let's see if this also causes an eideticker regression in timecube or nytimes-scroll. If it does cause regressions then the regression is testCheck2 is real. If not, we'll just take the new testCheck2 results as the new baseline.

I'll be watching the eideticker results in inbound.
Attachment #8524629 - Flags: review?(mark.finkle)
Comment on attachment 8524629 [details] [diff] [review]
Restore tab strip to its original height (r=mfinkle)

Looks OK, but we need to make sure eideticker tests are working before we land.
Attachment #8524629 - Flags: review?(mark.finkle) → review+
wlach, could you please run eideticker timecube and nytimes-scroll with this apk?
https://dl.dropboxusercontent.com/u/1187037/new-tablet-ui.apk
Flags: needinfo?(wlachance)
This won't actually work because we don't have the tablet UI on the galaxy nexus :(

Could we just do a manual visual inspection of the behaviour here? It should be obvious if a website like timecube is really misbehaving.
Flags: needinfo?(wlachance)
I think I'm ok just ignoring this regression for now. I tried it locally and everything seemed fine. I think it's likely some kind of problem with the test.
tracking-fennec: --- → ?
tracking-fennec: ? → ---
Comment on attachment 8524629 [details] [diff] [review]
Restore tab strip to its original height (r=mfinkle)

Approval Request Comment
[Feature/regressing bug #]: New tablet UI (bug 1014156)
[User impact if declined]: Too small padding on top of tab strip. Makes us look amateur.
[Describe test coverage new/current, TBPL]: Local only. Let's bake this in Nightly for a bit and then uplift. 
[Risks and why]: Low, just tweaks a couple of resource values.
[String/UUID change made/needed]: n/a
Attachment #8524629 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/4f72c81d5441
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Attachment #8524629 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8524629 [details] [diff] [review]
Restore tab strip to its original height (r=mfinkle)

> let's bake in Nightly for a bit and then uplift.
revert my approval
Attachment #8524629 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora?
Attachment #8524629 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed in builds:
- 38.0a1 2015-01-14;
- 37.0a2 2015-01-14;
- 36 Beta 1;
Device: Asus Transformer Tab (Android 4.0.3).
Status: RESOLVED → VERIFIED
If we wanted to run this locally and diagnose this issue properly...

<gbrown> capella: I ran testCheck2 locally -- https://wiki.mozilla.org/Mobile/Fennec/Android#talos updated
<gbrown> capella: for now, note that you will need to apply the patch from bug 1122701
You need to log in before you can comment on or make changes to this bug.