Add-ons preferences

VERIFIED FIXED in Firefox 13

Status

()

Firefox for Android
General
P2
normal
VERIFIED FIXED
6 years ago
10 months ago

People

(Reporter: fabrice, Assigned: mbrubeck)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

unspecified
Firefox 14
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox13 fixed, firefox14 verified, firefox15 verified, blocking-fennec1.0 beta+, fennec11+)

Details

Attachments

(4 attachments, 5 obsolete attachments)

(Reporter)

Description

6 years ago
Can we show them inline in the add-on manager?
(Reporter)

Updated

6 years ago
Blocks: 696518
Yes, that is my plan in bug 696532
Assignee: nobody → mark.finkle

Updated

6 years ago
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: 696532
OS: Linux → Android
Hardware: x86_64 → ARM
tracking-fennec: --- → 11+
Duplicate of this bug: 717065

Updated

5 years ago
Blocks: 467520
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.
Created attachment 589413 [details] [diff] [review]
WIP

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
Created attachment 589414 [details]
screenshot

Screenshot of Zippity add-on options
Created attachment 594056 [details] [diff] [review]
WIP 2

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
Keywords: fennecnative-releaseblocker

Updated

5 years ago
Keywords: fennecnative-releaseblocker → fennecnative-betablocker
blocking-fennec1.0: --- → beta+
Status: NEW → ASSIGNED
(Assignee)

Comment 9

5 years ago
Created attachment 600567 [details] [diff] [review]
WIP

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
Created attachment 601991 [details]
Attached are some layout adjustments, I believe it is difficult to get the exact coordinates so please match the layout as close as possible. Basically try to make everything align to something.
(Assignee)

Updated

5 years ago
Depends on: 732063
Created attachment 602014 [details]
H DPI Checkboxes
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.
(Assignee)

Updated

5 years ago
Depends on: 732265

Comment 14

5 years ago
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).
(Assignee)

Comment 15

5 years ago
(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.
(Assignee)

Comment 16

5 years ago
Created attachment 605470 [details] [diff] [review]
1. XBL bindings and basic styles

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)
Created attachment 605568 [details]
H DPI Checkboxes
Attachment #602014 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #605470 - Attachment description: 1/2: XBL bindings and basic styles → 1. XBL bindings and basic styles
(Assignee)

Comment 18

5 years ago
Created attachment 605586 [details] [diff] [review]
2. Style tweaks and checkbox images

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+
(Assignee)

Comment 20

5 years ago
(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.
(Assignee)

Comment 21

5 years ago
We might want to add full-row touch support to other setting types, as we already do for checkboxes...
(Assignee)

Comment 22

5 years ago
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
(Assignee)

Comment 23

5 years ago
https://hg.mozilla.org/mozilla-central/rev/8313b23498ad
https://hg.mozilla.org/mozilla-central/rev/7a85d32044db
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 24

5 years ago
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?
(Assignee)

Updated

5 years ago
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+

Updated

5 years ago
Attachment #605586 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Updated

5 years ago
Blocks: 737692
https://hg.mozilla.org/releases/mozilla-aurora/rev/20692069e82f
https://hg.mozilla.org/releases/mozilla-aurora/rev/3d1a7b3938d0
status-firefox13: --- → fixed
status-firefox14: --- → fixed
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
status-firefox14: fixed → verified
status-firefox15: --- → verified

Updated

5 years ago
Depends on: 762015
(Assignee)

Updated

5 years ago
Depends on: 735874
You need to log in before you can comment on or make changes to this bug.