Last Comment Bug 347327 - Implement mediatype support for output
: Implement mediatype support for output
Status: RESOLVED FIXED
: fixed1.8.0.12, fixed1.8.1.4
Product: Core
Classification: Components
Component: XForms (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: alexander :surkov
: Stephen Pride
Mentors:
http://www.w3.org/TR/xforms11/#render...
Depends on: 376817
Blocks:
  Show dependency treegraph
 
Reported: 2006-08-04 01:55 PDT by alexander :surkov
Modified: 2007-04-23 16:35 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.83 KB, application/xhtml+xml)
2006-08-05 14:11 PDT, alexander :surkov
no flags Details
patch (14.72 KB, patch)
2006-08-05 14:18 PDT, alexander :surkov
aaronr: review-
Details | Diff | Review
testcase2 (2.05 KB, application/xhtml+xml)
2006-08-09 11:24 PDT, alexander :surkov
no flags Details
patch2 (17.64 KB, patch)
2006-08-09 11:26 PDT, alexander :surkov
aaronr: review+
doronr: review+
Details | Diff | Review
typo patch (17.65 KB, patch)
2006-08-16 04:31 PDT, alexander :surkov
no flags Details | Diff | Review

Description alexander :surkov 2006-08-04 01:55:51 PDT
XForms 1.1 specs allow to specify mediatype for outputs. Do we have the bug on it already? Do we have any reason why we shouldn't support it?
Comment 1 alexander :surkov 2006-08-05 14:11:13 PDT
Created attachment 232362 [details]
testcase
Comment 2 alexander :surkov 2006-08-05 14:18:20 PDT
Created attachment 232363 [details] [diff] [review]
patch
Comment 3 alexander :surkov 2006-08-05 14:27:26 PDT
Patch add support only outputs that are bound to data of xsd:anyURI type. We should support "xsd:base64Binary" and "xsd:hexBinary" too. But I don't know how. Probably it requires image layout changing. Thoughts?
Comment 4 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2006-08-06 00:59:37 PDT
Could we convert xsd:base64Binary and xsd:hexBinary
to data: urls?
Comment 5 aaronr 2006-08-07 12:13:31 PDT
(In reply to comment #0)
> XForms 1.1 specs allow to specify mediatype for outputs. Do we have the bug on
> it already? Do we have any reason why we shouldn't support it?
> 


This will be cool to have.  But like the other 1.1 patches we've done and are being worked on, once the approvals are given this will only be checked into the trunk.  Until the 1.1 spec reaches recommendation.

I think this will be the 4th 1.1 item undertaken.  Not bad!
Comment 6 aaronr 2006-08-07 13:04:53 PDT
Comment on attachment 232363 [details] [diff] [review]
patch

>Index: extensions/xforms/nsXFormsMediatypeElement.cpp
>===================================================================
>RCS file: extensions/xforms/nsXFormsMediatypeElement.cpp
>diff -N extensions/xforms/nsXFormsMediatypeElement.cpp
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ extensions/xforms/nsXFormsMediatypeElement.cpp	5 Aug 2006 21:15:57 -0000
>@@ -0,0 +1,93 @@
>+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>+/* ***** BEGIN LICENSE BLOCK *****
>+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
>+ *
>+ * The contents of this file are subject to the Mozilla Public License Version
>+ * 1.1 (the "License"); you may not use this file except in compliance with
>+ * the License. You may obtain a copy of the License at
>+ * http://www.mozilla.org/MPL/
>+ *
>+ * Software distributed under the License is distributed on an "AS IS" basis,
>+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
>+ * for the specific language governing rights and limitations under the
>+ * License.
>+ *
>+ * The Original Code is Mozilla XForms support.
>+ *
>+ * The Initial Developer of the Original Code is
>+ * Alexander Surkov.
>+ * Portions created by the Initial Developer are Copyright (C) 2006
>+ * the Initial Developer. All Rights Reserved.
>+ *
>+ * Contributor(s):
>+ *  Alexander Surkov <surkov.alexander@gmail.com> (original author)
>+ *
>+ * Alternatively, the contents of this file may be used under the terms of
>+ * either the GNU General Public License Version 2 or later (the "GPL"), or
>+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
>+ * in which case the provisions of the GPL or the LGPL are applicable instead
>+ * of those above. If you wish to allow use of your version of this file only
>+ * under the terms of either the GPL or the LGPL, and not to allow others to
>+ * use your version of this file under the terms of the MPL, indicate your
>+ * decision by deleting the provisions above and replace them with the notice
>+ * and other provisions required by the GPL or the LGPL. If you do not delete
>+ * the provisions above, a recipient may use your version of this file under
>+ * the terms of any one of the MPL, the GPL or the LGPL.
>+ *
>+ * ***** END LICENSE BLOCK ***** */
>+
>+/**
>+ * Implementation of the XForms \<mediatype\> element.
>+ */
>+
>+#include "nsXFormsDelegateStub.h"
>+#include "nsIDOM3Node.h"
>+
>+class nsXFormsMediatypeElement : public nsXFormsDelegateStub
>+{
>+public:
>+  // nsIXFormsDelegate
>+  NS_IMETHOD GetValue(nsAString& aValue);
>+
>+  nsXFormsMediatypeElement();
>+
>+#ifdef DEBUG_smaug
>+  virtual const char* Name() { return "mediatype"; }
>+#endif
>+};
>+
>+nsXFormsMediatypeElement::nsXFormsMediatypeElement():
>+  nsXFormsDelegateStub(NS_LITERAL_STRING("mediatype"))
>+{
>+}
>+
>+NS_IMETHODIMP
>+nsXFormsMediatypeElement::GetValue(nsAString& aValue)
>+{
>+  // The order of precedence for determining the mediatype is:
>+  //   single node binding, inline text
>+
>+  nsXFormsDelegateStub::GetValue(aValue);
>+  if (aValue.IsVoid()) {
>+    NS_ENSURE_STATE(mElement);
>+
>+    nsCOMPtr<nsIDOM3Node> inner(do_QueryInterface(mElement));
>+    if (inner) {
>+      inner->GetTextContent(aValue);
>+    }
>+  }
>+
>+  return NS_OK;
>+}
>+
>+NS_HIDDEN_(nsresult)
>+NS_NewXFormsMediatypeElement(nsIXTFElement **aResult)
>+{
>+  *aResult = new nsXFormsMediatypeElement();
>+  if (!*aResult)
>+    return NS_ERROR_OUT_OF_MEMORY;
>+
>+  NS_ADDREF(*aResult);
>+  return NS_OK;
>+}
>+

I think we probably need to override ::Bind and/or ::Refresh here.  They'll get called if someone uses the DOM to change the binding attribute on the xf:mediatype element.  So we'll need to reflect that change in the output's @mozType:mediatype.




>Index: extensions/xforms/resources/content/xforms-xhtml.xml
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/xforms-xhtml.xml,v
>retrieving revision 1.12
>diff -u -8 -p -r1.12 xforms-xhtml.xml
>--- extensions/xforms/resources/content/xforms-xhtml.xml	2 Aug 2006 18:29:37 -0000	1.12
>+++ extensions/xforms/resources/content/xforms-xhtml.xml	5 Aug 2006 21:15:57 -0000
>@@ -106,16 +106,42 @@
>           return this.ownerDocument.
>             getAnonymousElementByAttribute(this, "anonid", "control");
>         </body>
>       </method>
>     </implementation>
>   </binding>
> 
> 
>+  <!-- OUTPUT: <MEDIATYPE="image/*", TYPE="xsd:anyURI"> -->
>+  <binding id="xformswidget-output-mediatype-anyURI"
>+           extends="chrome://xforms/content/xforms.xml#xformswidget-output-base">
>+    <content>
>+      <children includes="label"/>
>+      <html:img class="xf-value" anonid="control"/>
>+      <children/>
>+    </content>
>+
>+    <implementation>
>+      <method name="getControlElement">
>+        <body>
>+          return {
>+            __proto__: this.ownerDocument.
>+              getAnonymousElementByAttribute(this, "anonid", "control"),
>+
>+            set value(val) {
>+              this.src = val;
>+            }
>+          };
>+        </body>
>+      </method>
>+    </implementation>
>+  </binding>
>+
>+
>   <!-- LABEL: <DEFAULT> -->
>   <binding id="xformswidget-label"
>            extends="chrome://xforms/content/xforms.xml#xformswidget-label-base">
>     <content>
>       <html:span anonid="implicitcontent"/>
>       <html:span anonid="explicitcontent"><children/></html:span>
>     </content>
> 
>Index: extensions/xforms/resources/content/xforms-xul.xml
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/xforms-xul.xml,v
>retrieving revision 1.7
>diff -u -8 -p -r1.7 xforms-xul.xml
>--- extensions/xforms/resources/content/xforms-xul.xml	2 Aug 2006 18:29:37 -0000	1.7
>+++ extensions/xforms/resources/content/xforms-xul.xml	5 Aug 2006 21:15:57 -0000
>@@ -90,16 +90,42 @@
>           return this.ownerDocument.
>             getAnonymousElementByAttribute(this, "anonid", "control");
>         </body>
>       </method>
>     </implementation>
>   </binding>
> 
> 
>+  <!-- OUTPUT: <MEDIATYPE="image/*", TYPE="xsd:anyURI"> -->
>+  <binding id="xformswidget-output-meidatype-anyURI"
>+           extends="chrome://xforms/content/xforms.xml#xformswidget-output-base">
>+    <content>
>+      <children includes="label"/>
>+      <xul:image class="xf-value" anonid="control"/>
>+      <children/>
>+    </content>
>+
>+    <implementation>
>+      <method name="getControlElement">
>+        <body>
>+          return {
>+            __proto__: this.ownerDocument.
>+              getAnonymousElementByAttribute(this, "anonid", "control"),
>+
>+            set value(val) {
>+              this.src = val;
>+            }
>+          };
>+        </body>
>+      </method>
>+    </implementation>
>+  </binding>
>+
>+
>   <!-- LABEL: <DEFAULT> -->
>   <binding id="xformswidget-label"
>            extends="chrome://xforms/content/xforms.xml#xformswidget-label-base">
>     <content>
>       <xul:deck anonid="contentswitcher" flex="1" selectedIndex="1">
>         <xul:label anonid="implicitcontent"/>
>         <xul:label><children/></xul:label>
>       </xul:deck>
>Index: extensions/xforms/resources/content/xforms.css
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/xforms.css,v
>retrieving revision 1.40
>diff -u -8 -p -r1.40 xforms.css
>--- extensions/xforms/resources/content/xforms.css	2 Aug 2006 18:29:37 -0000	1.40
>+++ extensions/xforms/resources/content/xforms.css	5 Aug 2006 21:15:57 -0000
>@@ -104,16 +104,21 @@ html|*:root select1[appearance='compact'
> html|*:root select1[appearance='full'] choices {
>   display: none;
> }
> 
> message, alert, help {
>   display: none;
> }
> 
>+mediatype {
>+  -moz-binding: url('chrome://xforms/content/xforms.xml#xformswidget-mediatype');
>+  display: none;
>+}
>+
> html|*:root message[level="ephemeral"], html|*:root hint {
>   position: absolute;
>   z-index: 2147481647;
>   visibility: hidden;
>   top: 0px;
>   left: 0px;
>   width: 0px;
>   height: 0px;
>@@ -192,16 +197,29 @@ html|*:root output[mozType|type="http://
> xul|*:root output[mozType|type="http://www.w3.org/2001/XMLSchema#date"][appearance="full"] {
>   -moz-binding: url('chrome://xforms/content/xforms-xul.xml#xformswidget-output-date-full');
> }
> xul|*:root output[mozType|type="http://www.w3.org/2001/XMLSchema#date"][appearance="full"]
>     xul|box[mozType|calendar] {
>   -moz-binding: url('chrome://xforms/content/widgets-xul.xml#calendar-full');
> }
> 
>+  /* output mediatype="image/*", type="xsd:anyURI" */
>+html|*:root output[mozType|type="http://www.w3.org/2001/XMLSchema#anyURI"][mediatype^="image"],
>+html|*:root output[mozType|type="http://www.w3.org/2001/XMLSchema#anyURI"][mozType|mediatype^="image"]
>+{
>+  -moz-binding: url('chrome://xforms/content/xforms-xhtml.xml#xformswidget-output-mediatype-anyURI');
>+}
>+
>+xul|*:root output[mozType|type="http://www.w3.org/2001/XMLSchema#anyURI"][mediatype^="image"],
>+xul|*:root output[mozType|type="http://www.w3.org/2001/XMLSchema#anyURI"][mozType|mediatype^="image"]
>+{
>+  -moz-binding: url('chrome://xforms/content/xforms-xhtml.xml#xformswidget-output-mediatype-anyURI');
>+}
>+
> /* range widgets */
> range {
>   -moz-binding: url('chrome://xforms/content/range.xml#xformswidget-range');
> }
> 

You should probably put in XXX's here so that we use derived types once the typeList patch is in.  Or open a bug for that support.


> /* input widgets */
> 
>   /* input */
>Index: extensions/xforms/resources/content/xforms.xml
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/xforms.xml,v
>retrieving revision 1.41
>diff -u -8 -p -r1.41 xforms.xml
>--- extensions/xforms/resources/content/xforms.xml	2 Aug 2006 18:29:37 -0000	1.41
>+++ extensions/xforms/resources/content/xforms.xml	5 Aug 2006 21:15:57 -0000

>+
>+  <!-- MEDIATYPE -->
>+  <binding id="xformswidget-mediatype" extends="#xformswidget-base">
>+    <implementation implements="nsIXFormsUIWidget">
>+      <method name="refresh">
>+        <body>
>+          this.setMediatype();
>+        </body>
>+      </method>
>+
>+      <method name="setMediatype">
>+        <body>
>+        <![CDATA[
>+          var parent = this.parentNode;
>+          if (parent.namespaceURI == this.XFORMS_NS &&
>+              parent.localName == "output" &&
>+              !parent.hasAttribute("mediatype")) {
>+            parent.setAttributeNS(this.MOZTYPE_NS, "mediatype",
>+                                  this.stringValue);
>+          }
>+        ]]>
>+        </body>
>+      </method>
>+    </implementation>
>+

I'd say that if we are going to have a moztype:mediatype attribute, that it should always be set if there is a mediatype attribute or if there is a mediatype element affecting the control.  Then control authors only have to look in one place for styling, etc.
Comment 7 aaronr 2006-08-07 13:33:29 PDT
(In reply to comment #4)
> Could we convert xsd:base64Binary and xsd:hexBinary
> to data: urls?
> 

I don't know much about them, but it looks like only base64 is supported that way?  http://www.faqs.org/rfcs/rfc2397.html
Comment 8 alexander :surkov 2006-08-08 02:48:56 PDT
> I think we probably need to override ::Bind and/or ::Refresh here.  They'll get
> called if someone uses the DOM to change the binding attribute on the
> xf:mediatype element.  So we'll need to reflect that change in the output's
> @mozType:mediatype.

I though nsXFormsDelegate reflects properly on such changes, for example it calls nsIXFormsUIWidget::refresh() method. Am I wrong?

> You should probably put in XXX's here so that we use derived types once the
> typeList patch is in.  Or open a bug for that support.

If you'd like then I'll add xxx comment for sure but I guess somebody who will fix bug 316691 should take it into account without additional comments.

I'd say that if we are going to have a moztype:mediatype attribute, that it
should always be set if there is a mediatype attribute or if there is a
mediatype element affecting the control.  Then control authors only have to
look in one place for styling, etc.

Yes, probably it's good for styling, at least it will work for us (patch can be shorter on two lines code) :).
Comment 9 aaronr 2006-08-08 10:07:27 PDT
(In reply to comment #8)
> > I think we probably need to override ::Bind and/or ::Refresh here.  They'll get
> > called if someone uses the DOM to change the binding attribute on the
> > xf:mediatype element.  So we'll need to reflect that change in the output's
> > @mozType:mediatype.
> 
> I though nsXFormsDelegate reflects properly on such changes, for example it
> calls nsIXFormsUIWidget::refresh() method. Am I wrong?
> 

You are right.  My bad.
Comment 10 alexander :surkov 2006-08-09 11:24:04 PDT
Created attachment 232942 [details]
testcase2
Comment 11 alexander :surkov 2006-08-09 11:26:55 PDT
Created attachment 232943 [details] [diff] [review]
patch2
Comment 12 aaronr 2006-08-15 16:12:13 PDT
Comment on attachment 232943 [details] [diff] [review]
patch2

Well, you have an issue if the mime type isn't a valid value or format.  If I make the value of the bound node to be "image/foo/cat", I'm guessing that I should not see an image but I do.  And even if you properly figure out that this is a problem, we probably need some kind of error.  Should probably also check with the WG to see if it will be a xforms error of some kind.

If you open a bug to address those two issues (can probably be the same bug), r=me
Comment 13 alexander :surkov 2006-08-15 19:12:18 PDT
(In reply to comment #12)

> If you open a bug to address those two issues (can probably be the same bug),
> r=me
> 

I filed bug 348797.
Comment 14 Doron Rosenberg (IBM) 2006-08-15 19:29:25 PDT
Comment on attachment 232943 [details] [diff] [review]
patch2

>Index: extensions/xforms/nsXFormsMediatypeElement.cpp
...
>+#include "nsXFormsDelegateStub.h"
>+#include "nsIDOM3Node.h"
>+
>+class nsXFormsMediatypeElement : public nsXFormsDelegateStub
>+{
>+public:
>+  // nsIXFormsDelegate
>+  NS_IMETHOD GetValue(nsAString& aValue);
>+
>+  nsXFormsMediatypeElement();
>+
>+#ifdef DEBUG_smaug
>+  virtual const char* Name() { return "mediatype"; }
>+#endif

Do we really need a new smaug ifdef? :)
Comment 15 alexander :surkov 2006-08-15 20:30:06 PDT
(In reply to comment #14)

> >+#ifdef DEBUG_smaug
> >+  virtual const char* Name() { return "mediatype"; }
> >+#endif
> 
> Do we really need a new smaug ifdef? :)
> 

Good question :). Smaug, do we need your debug code?
Comment 16 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2006-08-16 00:14:27 PDT
(In reply to comment #15)
> 
> Good question :). Smaug, do we need your debug code?
> 

Perhaps we could change that to # ifdef DEBUG_XFORMS.
Comment 17 alexander :surkov 2006-08-16 00:31:40 PDT
(In reply to comment #16)
> (In reply to comment #15)
> > 
> > Good question :). Smaug, do we need your debug code?
> > 
> 
> Perhaps we could change that to # ifdef DEBUG_XFORMS.
> 

Ok, then I guess we should have separate bug for that since DEBUG_SMAUG instructions are very widespread.
Comment 18 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2006-08-16 01:50:13 PDT
Comment on attachment 232943 [details] [diff] [review]
patch2


>+  <!-- OUTPUT: <MEDIATYPE="image/*", TYPE="xsd:anyURI"> -->
>+  <binding id="xformswidget-output-meidatype-anyURI"
>+           extends="chrome://xforms/content/xforms.xml#xformswidget-output-base">

...

>+  <!-- OUTPUT: <MEDIATYPE="image/*", TYPE="xsd:base64Binary"> -->
>+  <binding id="xformswidget-output-meidatype-base64Binary"


Could you fix this typo 'meida'
Comment 19 alexander :surkov 2006-08-16 04:31:29 PDT
Created attachment 233987 [details] [diff] [review]
typo patch

Typo 'meida' fixed
Comment 20 aaronr 2007-04-07 15:28:39 PDT
This won't currently work on FF2 or FF.15 due to bug 376817 (xbl).
Comment 21 aaronr 2007-04-23 16:35:33 PDT
checked into 1.8 branch on 2007-04-12
checked into 1.8.0 branch on 2007-04-16

Note You need to log in before you can comment on or make changes to this bug.