Closed Bug 511627 Opened 10 years ago Closed 10 years ago

Implement a dialog system for fennec

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
fennec1.0b4

People

(Reporter: fabrice.desre, Assigned: fabrice.desre)

References

Details

Attachments

(1 file, 3 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5
Build Identifier: fennec 1.0b3

Create a non-windowed dialog system for fennec.

Reproducible: Always
Attached patch v1 (obsolete) — Splinter Review
This patch add support for dialogs loaded from remote files :
<dialog xmlns="$XUL_NS" 
        onload="..." 
        onclose="..."
        script="js file to load in dialog scope"
        closebutton="true|false">
 <label value="hello world dialog !"/>
 <button class="button-dark" oncommand=""/>
</dialog>

Opening a dialog is done using the openDialog(uri, args) available in browser.js

For now all dialogs are modal.
Attachment #395564 - Flags: review?(mark.finkle)
Blocks: 489423
Attached patch v1.1 (obsolete) — Splinter Review
removed nsIPrompt implementation from this patch, and included jar.mn
Attachment #395564 - Attachment is obsolete: true
Attachment #395564 - Flags: review?(mark.finkle)
Attachment #395568 - Flags: review?(mark.finkle)
Attachment #395568 - Flags: review?(mark.finkle) → review-
Comment on attachment 395568 [details] [diff] [review]
v1.1

>diff -r 30e5022551d0 chrome/content/bindings/dialog.xml

>+      <constructor><![CDATA[
>+        if (!this.hasAttribute(orient))

"orient"  ?

>diff -r 30e5022551d0 chrome/content/browser.js

>+  // create a full-screen semi-opaque box as a background 
>+  let back = document.createElement("box");
>+  back.setAttribute("align", "center");
>+  back.setAttribute("pack", "center");
>+  back.setAttribute("style", "background-color: rgba(128,128,128,0.5)");

I'm think we should add these to a CSS rule and not add them explicitly. Just add a .className = "modal-block"

>+  // should allow non-modal dialogs, but doesn't work
>+  //dialog = parent.insertBefore(document.importNode(doc, true), selectContainer);

Just remove for now
Attached patch v1.2 (obsolete) — Splinter Review
Addresses issues in comment #3, and adds a "modal" parameter to open dialog to avoid a signature change when the non-modal behavior will be fixed.
Attachment #395568 - Attachment is obsolete: true
Attachment #396763 - Flags: review?(mark.finkle)
Comment on attachment 396763 [details] [diff] [review]
v1.2

>diff -r 4f92ef5a340a chrome/content/browser.css

>+.modal-block {
>+    -moz-box-align: center;
>+    -moz-box-pack: center;
>+    background-color: rgba(128,128,128,0.5);
>+}

Let's put this in the themes/hildon/browser.css and themes/wince/browser.css files instead

>+function openDialog(src, modal, arguments) {

Let's skip the modal parameter for now. Everything will just open as modal. We can fix later.

r+ with those nits fixed
Attachment #396763 - Flags: review?(mark.finkle) → review+
> 
> Let's skip the modal parameter for now. Everything will just open as modal. We
> can fix later.

Skipping the modal parameter is probably ok for the "core" of Fennec, but extensions developers will also be impacted. That was my main reason to add it now even if not supported.
Attached patch v1.3Splinter Review
Addresses issues in comment #5
Attachment #396763 - Attachment is obsolete: true
Attachment #397602 - Flags: review?(mark.finkle)
Assignee: nobody → fabrice.desre
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #397602 - Flags: review?(mark.finkle) → review+
pushed: https://hg.mozilla.org/mobile-browser/rev/c21d7d3748a8

renamed "openDialog" -> "importDialog"
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → B4
You need to log in before you can comment on or make changes to this bug.