507391 - noscript addon causes select box rendering to fail in fennec

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Extension Compatibility, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

in testing a latest nightly I didn't see the <select> input type rendering as expected.  After further investigation, I found out the root cause:
 - noscript extension was blocking the javascript (on my maemo device)
 - preference for enabling javascript was turned to "off" (on my desktop)

These are things that most people don't think about when using a browser.  If there is a way to do the <select> rendering without javascript that would be ideal.  Otherwise we should be very careful about what functionality we add to fennec that utilizes javascript and keep that well documented for add on developers and FAQ

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

this is also causing the URL bar to lose focus while trying to type in characters on the latest builds (8/11/09 nightly for maemo)

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

I found out that I can reproduce my url bar problem if noscript is disabled, so the only issue with noscript is the <select> tags.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #2)
> I found out that I can reproduce my url bar problem if noscript is disabled, so
> the only issue with noscript is the <select> tags.

Now the problem should come with a all the elements handle by the Form Assistant.

I'm wondering if we should handle the select & co hook in the mousedown dispatch in browser.js (http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#1550) which can be my preferred method because this way a website can not trigger the Form Assistant against the will of the user.

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: wip

This implement the above idea. but this still need a few more work.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Same as previous but cleared

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 414509 [details] [diff] [review]
Patch

>diff -r cb80eae695c7 chrome/content/bindings.xml
>--- a/chrome/content/bindings.xml	Tue Nov 24 13:59:35 2009 -0800
>+++ b/chrome/content/bindings.xml	Wed Nov 25 18:06:13 2009 +0100
>@@ -893,64 +893,16 @@
>       
>           this.focus();
>           SelectHelper.show(this);
>         ]]>
>       </handler>
>     </handlers>
>   </binding>
> 
>-  <binding id="chrome-input">
>-    <content>
>-      <children />
>-    </content>
>-    <handlers>
>-      <handler event="click" button="0">
>-        <![CDATA[
>-          var showEvent = document.createEvent("Events");
>-          showEvent.initEvent("UIShowForm", true, false);
>-          this.dispatchEvent(showEvent);
>-        ]]>
>-      </handler>
>-    </handlers>
>-  </binding>
>-
>-  <binding id="chrome-select">
>-    <content>
>-      <children />
>-    </content>
>-
>-    <implementation>
>-      <property name="selectElement"
>-                onget="return this.QueryInterface(Components.interfaces.nsISelectElement);"
>-                readonly="true"/>
>-    </implementation>
>-
>-    <handlers>
>-      <handler event="mousedown" button="0" phase="capturing">
>-        <![CDATA[
>-          event.stopPropagation();
>-          event.preventDefault();
>-        ]]>
>-      </handler>
>-
>-      <handler event="click" button="0">
>-        <![CDATA[
>-          let options = this.options;
>-          if (options.length == 0)
>-            return;
>-      
>-          var showEvent = document.createEvent("Events");
>-          showEvent.initEvent("UIShowForm", true, false);
>-          this.dispatchEvent(showEvent);
>-        ]]>
>-      </handler>
>-    </handlers>
>-  </binding>
>-
>   <binding id="chrome-select-option">
>     <content orient="horizontal" flex="1">
>       <xul:image anonid="check"/>
>       <xul:label anonid="label" xbl:inherits="value=label"/>
>     </content>
> 
>     <implementation>
>       <property name="selected">
>diff -r cb80eae695c7 chrome/content/browser-ui.js
>--- a/chrome/content/browser-ui.js	Tue Nov 24 13:59:35 2009 -0800
>+++ b/chrome/content/browser-ui.js	Wed Nov 25 18:06:13 2009 +0100
>@@ -1240,16 +1240,24 @@ var FormHelper = {
>   zoom: function formHelperZoom(aElement) {
>     let zoomLevel = Browser._getZoomLevelForElement(aElement);
>     zoomLevel = Math.min(Math.max(kBrowserFormZoomLevelMin, zoomLevel), kBrowserFormZoomLevelMax);
> 
>     let elRect = this._getRectForElement(aElement);
>     let zoomRect = Browser._getZoomRectForPoint(elRect.center().x, elRect.y, zoomLevel);
> 
>     Browser.setVisibleRect(zoomRect);
>+  },
>+
>+  showUIFor: function(aElement) {

canShowUIFor() ?  since we are not actually showing anything here.

>-    let selectElement = this._control.wrappedJSObject.selectElement;
>+    let selectElement = this._control.QueryInterface(Components.interfaces.nsISelectElement);

Ci

>diff -r cb80eae695c7 chrome/content/browser.js

>+        if (FormHelper.showUIFor(element)) {
>+          let showEvent = element.ownerDocument.createEvent("Events");
>+          showEvent.initEvent("UIShowForm", true, false);
>+          element.dispatchEvent(showEvent);
>+          return;

Why do we need to fire the event anymore? Can't we just call the FormHelper directly now? Like we do for <menulist> elements.

Almost ready!

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

Adressed comments!

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
https://hg.mozilla.org/mobile-browser/rev/6c6ba58c1d7f

Comment 9

[Aakash Desai [:aakashd]]

verified FIXED on builds (using the mobile support site):
Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2b5pre) Gecko/20091130 Firefox/3.6b5pre Fennec/1.0b6pre

and

Mozilla/5.0 (X11; U; Linux armv6l; Nokia N8xx; en-US; rv:1.9.3a1pre) Gecko/20091130 Firefox/3.7a1pre Fennec/1.0b5