Closed Bug 818056 Opened 12 years ago Closed 12 years ago

[Settings] Panel links in the Settings app do not work properly

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect, P2)

x86_64
Linux

Tracking

(blocking-basecamp:+, firefox18 unaffected, firefox20 fixed)

RESOLVED FIXED
blocking-basecamp +
Tracking Status
firefox18 --- unaffected
firefox20 --- fixed

People

(Reporter: hub, Assigned: kgrandon)

References

Details

(Keywords: regression)

Attachments

(4 files, 4 obsolete files)

I wonder how much this is related to bug 817036, but:

Go to Settings
Tap Cellular & Data
Tap APN Settings

I currently get a "MMS Settings" pane with very bug info 
OK does nothing
Back does nothing

This only solution is to go back to the home screen and kill the settings app.

gaia is at 73f6968811b8b660e14e46924b9b40704bd9ddab
I did a repo sync and rebuilt clean.
Works for me. Note that there’s no "MMS Settings" panel yet…
(In reply to Hub Figuiere [:hub] from comment #0)
> I currently get a "MMS Settings" pane with very bug info 

That doesn’t really help. Would you provide a screenshot please?
Attached image screenshot
Hmm, strange. Can’t reproduce yet with the latest Gaia on my Otoro.
(In reply to Hub Figuiere [:hub] from comment #0)

> I currently get a "MMS Settings" pane with very bug info 

I meant "with very bogus info"


But here is what is worse.

I tried again and now I get the "Media Storage" preferences - that I used minutes ago to be able to copy that screenshot. Looks like the problem is more that the Settings app is a bit confused in what to display.
It is on Unagi in case that maters.
[Build]
BuildID=20121204094949
Milestone=20.0a1
Just rebuilt Gaia 18 and I still can’t reproduce the bug. I assume it’s specific to Nightly… *sigh*

Whatever this is, I hope it won’t land to Gecko 18. I’ve just updated the bug title.
Summary: No more APN settings → Panel links in the Settings app do not work properly
I rebuilt using gecko-18 that |./repo sync| does NOT checkout and I can't reproduce anymore.
I can reproduce this bug in Gecko 20 (Firefox Nightly, desktop). This looks realy bad…
Severity: normal → major
I can reproduce this on my Nexus S running B2G with current trunk B2G. I did a bisect on Gaia and it came up with the following as the bad commit:

e427943d5a26852622e36010a0300654a6ce4d69 is the first bad commit
commit e427943d5a26852622e36010a0300654a6ce4d69
Author: Tim Taubert <tim@timtaubert.de>
Date:   Mon Dec 3 16:00:10 2012 +0100

    Bug 817181 - [Settings] Panes are scrollable horizontally but they shouldn't be
With that commit reverted, I see "MMS Settings" briefly flash but then APN settings appears. With that commit I only see MMS Settings, the APN settings never appears.
Thanks for the bisect Chris! I assume we’re still speaking of Gecko 20 here, not Gecko 18?
CC’ing Tim and hoping he has another nice idea to work around this…
So latest Gaia works with Gecko 18 but not with Gecko 20, right? I don't have a trunk checkout, looks like I should start building...
(In reply to Fabien Cazenave [:kaze] from comment #13)
> Thanks for the bisect Chris! I assume we’re still speaking of Gecko 20 here,
> not Gecko 18?

I tested latest B2G trunk. That's Gaia commit 9bbbd62442a060a8f65db0a3014a94511c1fabb3 and gecko commit 3114cafbfead4cf21bf8e1a06a62f3519a9f79d9. B2G trunk was 2ab30e7fbd40708525a287b3d58c545af73d0aa9.
After firing 4 bugs, I found out the similar issue happens in most of our settings functions, so I stop firing others since all of them might be due to the same root cause.
Summary: Panel links in the Settings app do not work properly → [Settings] Panel links in the Settings app do not work properly
I try to collect the related settings bugs which are all fired recently and seem to have the same root cause. Please correct me if I'm wrong. Thanks!
Is this happening on beta?, I saw this in m-i, m-c and m-a, but not in beta.
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #19)
> Is this happening on beta?, I saw this in m-i, m-c and m-a, but not in beta.

No, beta doesn't seem to be affected.
Depends on: 797411
I took a look at #819247, and I have a workaround for the panels not painting properly. I think it's probably some deep gecko bug, but I have a workaround in the meantime. I'll take this to submit my pull request, and you guys can tell me what you think.

I can also perhaps put together some test cases if someone from platform wants to take a look.

Also 819247 is blocking, should this be also?
Assignee: nobody → kgrandon
blocking-basecamp: --- → ?
Attachment #690179 - Flags: review?
not blocking on META bug
blocking-basecamp: ? → ---
Keywords: meta
Comment on attachment 690179 [details]
Github pull request pointer

This PR is the same as bug 819247, I'd like to review here because this is the root cause issue.
Attachment #690179 - Flags: review? → review?(ehung)
No longer depends on: 819247
I believe some issues on the depends list don't really related to this issue, I'm  sorting them out.
This is probably not a meta issue, but a Gecko regression because the problem only happens on Gecko 20. Tim is investigating it.
Keywords: meta
No longer depends on: 817483
(In reply to Evelyn Hung [:evelyn] from comment #25)
> I believe some issues on the depends list don't really related to this
> issue, I'm  sorting them out.
> This is probably not a meta issue, but a Gecko regression because the
> problem only happens on Gecko 20. Tim is investigating it.

Thanks Evelyn for the double checks. :)
Attached file WIP (obsolete) —
I found that the underlining issue is bug 803170, which it's workaround is being void by change made in bug 817181. I update the workaround so the problem described in this bug will go away.

Interactions between workaround"s" makes me sad.
No longer depends on: 818550
No longer depends on: 818834
No longer depends on: 818880
No longer depends on: 819226
No longer depends on: 819231
No longer depends on: 819233
No longer depends on: 819235
No longer depends on: 819237
No longer depends on: 819651
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #28)
> I found that the underlining issue is bug 803170, which it's workaround is
> being void by change made in bug 817181. I update the workaround so the
> problem described in this bug will go away.
> 
> Interactions between workaround"s" makes me sad.

Can’t agree more. The *real* root cause that we’re working around is bug 797411 — I’ve nominated it for BB+ but it’s been minused twice.
No longer depends on: 817992
(In reply to Evelyn Hung [:evelyn] from comment #25)
> This is probably not a meta issue, but a Gecko regression because the
> problem only happens on Gecko 20.

Agreed, and re-nominating this bug for BB+ as it’s not a meta-bug.
No longer depends on: 817997
I vote for bb+ because it blocks many people's (especially platform dev) daily work, although it's not happened on our target Gecko build.
BTW, I'm done the sorting. Sorry for spam everyone.
blocking-basecamp: --- → ?
Attached file WIP (obsolete) —
This is the correct file. Oops.
Attachment #690767 - Attachment is obsolete: true
Attached file WIP v2 (obsolete) —
WIP v2 here attempts to hide the unseen panel with visibility: hidden, and show them with visibility transition during transition (see bug 803409 comment 2). I am not sure if this will stop Otoro from flashing when switching panels.
Attached file WIP v2.1 (obsolete) —
previous panel should also be hidden.
Attachment #690791 - Attachment is obsolete: true
Comment on attachment 690179 [details]
Github pull request pointer

Kevin, I tested your patch, it work good. If we can fix the hard-code 400 milliseconds by using CSS tricks, then I think it's a good workaround for this issue.
I'll pending the patch first, feel free to request my review if any updates.
Thanks.
Attachment #690179 - Flags: review?(ehung)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #42)
> Created attachment 690794 [details]
> WIP v2.1
> 
> previous panel should also be hidden.

Tim, The WIP v2 is still not good enough, the screen flickers while transition.
Attached patch WIP v1.1Splinter Review
I found that I should revert the transform during the uninit phase; if not, opening the settings app with a hash will put the viewport somewhere not (0, 0). This updates WIP v1.0 patch.
Attachment #690779 - Attachment is obsolete: true
Attached file WIP v2.2
See previous comment. Again, the only difference between v1 and v2 is the visibility part. If that doesn't the flash during panel switching then we could ditch v2 and work on something else on top of v1.

I don't have an working m-c Gecko on the phone with me, so all tests is being done on Firefox Nightly only :'(
Attachment #690794 - Attachment is obsolete: true
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #46)
> Created attachment 690803 [details]
> WIP v2.2
> 
> See previous comment. Again, the only difference between v1 and v2 is the
> visibility part. If that doesn't the flash during panel switching then we
> could ditch v2 and work on something else on top of v1.
Tested on my m-c build. It looks smoothly when the panel is the first time loaded, but flashes (like jumps a little) after the second time slip-in. (looks very like the WIP v2.1)  :'(
I suggest we stop tweaking CSS issue here if this fix won't affect our target Gecko build. Just make the function works so it won't block everybody's work on Gecko 20.

Tim, could you submit a PR so I can r+ and merge it?

Kevin, is it good for you?
Again, the root cause is bug 797411. I understand if you’re landing another workaround, but please make sure it does not make the performance worse on Gecko 18.
blocking-basecamp: ? → +
Priority: -- → P2
Comment on attachment 690179 [details]
Github pull request pointer

I'd much rather fix 797411 if we can get it re-nominated, if we just want a quick fix to unblock developers, that's fine by me too.

This pull request is still open, so I'll just add the review flag here. Feel free to take this one or Tim's.
Attachment #690179 - Flags: review?(ehung)
Comment on attachment 690179 [details]
Github pull request pointer

r=me. r+ with comments address.

Kevin, I'd like to land your patch for this workaround. Please address my comment in your PR and then I can merge it. Thanks, great job!
Attachment #690179 - Flags: review?(ehung) → review+
kaze, I tested and evaluate both patches(Tim's and Kevin's), I think Kevin's patch is better on visual and style consistency, although it really costs on adding/removing event listeners at every panel switching. But I *feel* it won't affect response time, thus I think it's okay to apply this patch. 
any comments?
Patch landed, thanks for the quick review.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Kevin, please attach commit number when you close issues.
https://github.com/mozilla-b2g/gaia/commit/17aa182fc29a068fef949d3108d13f2c3fc07c54
Blocks: 818070
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: