Closed Bug 696533 Opened 8 years ago Closed 8 years ago

Add-ons preferences

Categories

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

ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 14
Tracking Status
firefox13 --- fixed
firefox14 --- verified
firefox15 --- verified
blocking-fennec1.0 --- beta+
fennec 11+ ---

People

(Reporter: fabrice, Assigned: mbrubeck)

References

(Depends on 1 open bug)

Details

Attachments

(4 files, 5 obsolete files)

Can we show them inline in the add-on manager?
Blocks: 696518
Yes, that is my plan in bug 696532
Assignee: nobody → mark.finkle
Priority: -- → P2
If an add-on has options we need to be able to show/hide them by tapping a button. If an add-on is disabled, for whatever reason, the button should be disabled too.
Blocks: aom
OS: Linux → Android
Hardware: x86_64 → ARM
tracking-fennec: --- → 11+
Duplicate of this bug: 717065
Blocks: abp
mfinkle - Aren't we handling these in the secondary "add-ons details" screen for each add-on (vs. displayed after pressing a button)?
(In reply to Madhava Enros [:madhava] from comment #4)
> mfinkle - Aren't we handling these in the secondary "add-ons details" screen
> for each add-on (vs. displayed after pressing a button)?

Yes, that is the current design. Comment 2 is from Nov 21, 2011.
Attached patch WIP (obsolete) — Splinter Review
Adds basic support for add-on options:
* Ports <setting> and friends to HTML
* Styles the <setting>s in aboutAddons.css

To do:
* Needs to get the checkboxes a bit bigger
* Small tweaks the layout CSS
Attached image screenshot (obsolete) —
Screenshot of Zippity add-on options
Attached patch WIP 2 (obsolete) — Splinter Review
Just wanted to update with the latest WIP before handing off to Matt. This patch does show the options for an add-on:
* The checkbox rows need full-toggle support. The bindings still use "TapSingle" :(
* The "menulist" and "control" (with embedded "menulist") rows are busted because this is a XUL page and <menulist> is not supported. I tried looking for ways to add support for a <setting> that held <option>s and then have the XBL put those children in a <select> but it didn't work.

As I said, passing to Matt. I don't have the time to really work on this.
Attachment #589413 - Attachment is obsolete: true
Assignee: mark.finkle → mbrubeck
blocking-fennec1.0: --- → beta+
Status: NEW → ASSIGNED
Attached patch WIP (obsolete) — Splinter Review
Just a snapshot, building on Mark's previous patches.  I still need to do most of the theming work, and do something about menulists.
Attachment #589414 - Attachment is obsolete: true
Attachment #594056 - Attachment is obsolete: true
Depends on: 732063
Attached file H DPI Checkboxes (obsolete) —
Comment on attachment 600567 [details] [diff] [review]
WIP

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

Drive-by comments. I hate that we're going to end up with three implementations of the same thing to support, but there isn't much else we can do.

::: mobile/android/chrome/content/bindings/settings.xml
@@ +18,5 @@
> +      <html:div class="setting-label">
> +        <html:div class="preferences-title" xbl:inherits="xbl:text=title"/>
> +        <html:div class="preferences-description" xbl:inherits="xbl:text=desc">
> +          <children/>
> +        </html:div>

This <children> element (and the others like it) won't get used. The binding won't apply if the <setting> has text nodes, so we remove them and put the text content in the desc attribute (this doesn't actually happen on mobile yet, but I'm fixing it).

@@ +157,5 @@
> +          <children/>
> +        </html:div>
> +      </html:div>
> +      <html:div anonid="input-container" class="setting-input">
> +        <children includes="button|menulist"/>

That's not going to work without menulists.

@@ +211,5 @@
> +      </html:div>
> +      <html:div anonid="input-container" class="setting-input">
> +        <html:select size="1">
> +          <children includes="option" />
> +        </html:select>

For this to be compatible with our other implementation (including XUL mobile), you'd have to pick out the <menuitem>s from options.xul and replace them with <options>.

::: mobile/android/themes/core/aboutAddons.css
@@ +247,5 @@
> +
> +setting[type="bool"] {
> +  -moz-binding: url("chrome://browser/content/bindings/settings.xml#setting-fulltoggle-bool");
> +}
> +

To hide setting types we don't support, set display:none by default, and display:block on each of the types we do support. See bug 665515 and https://bugzilla.mozilla.org/attachment.cgi?id=600219&action=diff
Oh, I also meant to say that most of the JS implementation in the new bindings can be inherited, so you don't need a copy here. I didn't actually compare it with base bindings, but it looks mostly the same.
Depends on: 732265
With the dependency fixed now and the UI mockups done, is there anything blocking this one from moving forward (I know Matt could have been doing a bunch of FF 11 stuff).
(In reply to JP Rosevear [:jpr] from comment #14)
> With the dependency fixed now and the UI mockups done, is there anything
> blocking this one from moving forward (I know Matt could have been doing a
> bunch of FF 11 stuff).

No, nothing is blocking this.
This includes fully functional bindings and addresses darktrojan's feedback.  It actually removes most of the code from the earlier patches.

This patch has only very basic styles for layout; additional theming will be done in the next patch.
Attachment #600567 - Attachment is obsolete: true
Attachment #605470 - Flags: review?(mark.finkle)
Attached file H DPI Checkboxes
Attachment #602014 - Attachment is obsolete: true
Attachment #605470 - Attachment description: 1/2: XBL bindings and basic styles → 1. XBL bindings and basic styles
This adjusts the layout based on Patryk's mockup, and incorporates the checkbox images.

Still to do:  Better theming for integer prefs and menulists.
Attachment #605586 - Flags: review?(mark.finkle)
Attachment #605470 - Flags: review?(mark.finkle) → review+
Comment on attachment 605586 [details] [diff] [review]
2. Style tweaks and checkbox images

Since we are using the toolkit bindings for settings, we are using XUL widgets in the HTML, right? Those widget are all functional? The checkbox stuff in this patch makes it a better touch target. What of the other widgets?
Attachment #605586 - Flags: review?(mark.finkle) → review+
(In reply to Mark Finkle (:mfinkle) from comment #19)
> Since we are using the toolkit bindings for settings, we are using XUL
> widgets in the HTML, right? Those widget are all functional? The checkbox
> stuff in this patch makes it a better touch target. What of the other
> widgets?

Yeah, the widgets all work; bug 732265 added SelectHelper UI for menulists.

The "spinner" widgets for integer prefs are not really usable for mobile, and menulists need some theme love.  I'm working on both of those next.
We might want to add full-row touch support to other setting types, as we already do for checkboxes...
Pushed these to inbound; I'll file follow-up bugs for the remaining issues and nominate them blocking as appropriate.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8313b23498ad
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a85d32044db
Target Milestone: --- → Firefox 14
https://hg.mozilla.org/mozilla-central/rev/8313b23498ad
https://hg.mozilla.org/mozilla-central/rev/7a85d32044db
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment on attachment 605470 [details] [diff] [review]
1. XBL bindings and basic styles

[Approval Request Comment]
Mobile-only beta blocker.

String changes made by this patch: none
Attachment #605470 - Flags: approval-mozilla-aurora?
Attachment #605586 - Flags: approval-mozilla-aurora?
Comment on attachment 605470 [details] [diff] [review]
1. XBL bindings and basic styles

[Triage Comment]
Mobile only - approved for Aurora 13.
Attachment #605470 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #605586 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Blocks: 737692
Verified fixed on:

Firefox 15.0a1 (2012-04-30)
Firefox 14.0a2 (2012-04-30)

Device: Samsung Captivate
OS: Android 2.2
Status: RESOLVED → VERIFIED
Depends on: 762015
Depends on: 735874
You need to log in before you can comment on or make changes to this bug.