Closed Bug 1161157 Opened 5 years ago Closed 4 years ago

[Customizer Launcher] Enable developer mode confirm dialog

Categories

(Firefox OS Graveyard :: Gaia::Customizer, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: pdahiya, Assigned: pdahiya)

References

Details

Attachments

(1 file)

51 bytes, text/x-github-pull-request
justindarc
: review+
Details | Review
For user not having developer mode enabled, customizer launcher should show help dialog and if possible take user to settings app to enable developer mode on. Scope of this bug is to discuss and finalize how to handle this use case.

https://bugzilla.mozilla.org/show_bug.cgi?id=1160235#c1
https://bugzilla.mozilla.org/show_bug.cgi?id=1160235#c2
https://bugzilla.mozilla.org/show_bug.cgi?id=1160235#c3
Setting NI flag for Doug and Justin to help decide next steps.
Flags: needinfo?(jdarcangelo)
Flags: needinfo?(drs)
We'll have to solve this problem in several apps, so let's get the discussion going on in here. I'm needinfoing Mike for visibility of this topic. Mike, no info is actually needed, though your discussion and opinion is welcome.

I'm not optimistic about us being able to toggle developer mode on-device -- despite how great that would be for UX -- based on how the conversation in bug 1160235 is going. Remember that developer mode requires the device to be rooted, which no matter how we look at this, is not something we're guaranteed to have.

Thus, regardless of what happens in bug 1160235, we'll have to present information to the user about how to toggle developer mode, so that seems like a good start to me. I would suggest making an overlay like this:

"
Developer mode not enabled
---

To gain access to this and other exciting customization features, you must enable developer mode.

(Cancel) (More Info)
"

The "Cancel" button would close the app. The "More Info" button would just link to a web page, probably with a video, explaining how to enable developer mode, and what the risks are.

I think that it would be advantageous to turn this overlay into a Bower package, or perhaps even a WC, which every app that needs it can import. We will need someone to drive this, so I think that starting with the Customizer Launcher is a good place to prototype it.

The actual overlay itself definitely requires some UX input. Jacqueline, what are your thoughts?
Flags: needinfo?(mhenretty)
Flags: needinfo?(jsavory)
Flags: needinfo?(drs)
I'm on board with the method proposed in comment 2. It would be nice if the system app could have a way to know if the device was rooted, and be able to toggle developer mode directly (or through an activity to settings perhaps) if so. But I'm not sure how feasible that is.
Flags: needinfo?(mhenretty)
You don't need to root the device to have developer mode access, you just need a way to set a gecko pref.

One way to do it is to use the "reset and root my phone" feature of the settings app, and then webide can toggle any pref.
As per discussion https://bugzilla.mozilla.org/show_bug.cgi?id=1160235#c11, we are exposing the dev mode state read-only through the navigator.hasFeature() api (Bug 1161677).

CL will check for this feature and show enable developer mode confirm dialog described in #comment 2. 
Marking this bug  dependent on Bug 1161677
Depends on: 1161677
The discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1160235#c11, points enabling full dev tools mode from device by using Settings app activity. 

NI Doug to help if we have a bug open for it. If yes, we should mark dependency on that bug. Thanks!
Flags: needinfo?(drs)
See Also: → 1166105
Assignee: nobody → pdahiya
Marking dependency on Bug 1163889 to handle continue/next button from 'developer mode confirm dialog' to enable devtool mode.
Depends on: 1163889
Hi Justin
Please review attached web component for showing confirm dialog if dev perf on device is not enabled. Thanks!
Attachment #8610798 - Flags: review?(jdarcangelo)
Flags: needinfo?(drs)
Comment on attachment 8610798 [details] [review]
PR with fix of Bug 1161157

Punam, looks good! I left you some comments in the PR regarding the public API you're exposing on the WC. IMO, this component should try and take care of as much logic as possible on its own without requiring the containing app to make any API calls on the component. That's my only issue with this PR as the rest of the code looks good. Thanks!
Flags: needinfo?(jdarcangelo)
Attachment #8610798 - Flags: review?(jdarcangelo) → review-
(In reply to Justin D'Arcangelo [:justindarc] from comment #9)
> Comment on attachment 8610798 [details] [review]
> PR with fix of Bug 1161157
> 
> Punam, looks good! I left you some comments in the PR regarding the public
> API you're exposing on the WC. IMO, this component should try and take care
> of as much logic as possible on its own without requiring the containing app
> to make any API calls on the component. That's my only issue with this PR as
> the rest of the code looks good. Thanks!

Hi Justin, I have updated the PR with your feedback. WC is handling show/hide in createdCallback eliminating the need to call it from containing app. I have left isPrefEnabled public for apps such as Launcher that needs to check for pref setting before processing the bulk of app logic. Please review. Thanks!
Attachment #8610798 - Flags: review- → review?(jdarcangelo)
Comment on attachment 8610798 [details] [review]
PR with fix of Bug 1161157

Looks good! One nit I mentioned in the PR that you can address before you land (indentation).
Attachment #8610798 - Flags: review?(jdarcangelo) → review+
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Flags: needinfo?(jsavory)
You need to log in before you can comment on or make changes to this bug.