+++ This bug was initially created as a clone of Bug #904224 +++ +++ This bug was initially created as a clone of Bug #902168 +++ After the two landings from bug 904224, we're still seeing lots of forms.js logspam. https://tbpl.mozilla.org/php/getParsedLog.php?id=26488355&tree=B2g-Inbound 07:50:03 INFO - System JS : ERROR chrome://browser/content/forms.js:843 07:50:03 INFO - TypeError: encoder is null 07:50:03 INFO - ************************************************************ 07:50:03 INFO - * Call to xpconnect wrapped JSObject produced this error: * 07:50:03 INFO - [Exception... "'[JavaScript Error: "encoder is null" {file: "chrome://browser/content/forms.js" line: 843}]' when calling method: [nsIDOMEventListener::handleEvent]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0" data: yes] 07:50:03 INFO - ************************************************************ 07:50:20 INFO - System JS : ERROR chrome://browser/content/forms.js:756 07:50:20 INFO - TypeError: value is null Any volunteers? :-)

I should take the bug, but I run into problems setting up mochitest for b2g dev environment on b2g-desktop and the device. I'm trying to solve it. So please feel free to take the bug if you have time.

`TypeError: encoder is null` is introduced by bug 899999. We mistakely treat the <select> object as contentEditable element and then fail to get the encoder. `TypeError: value is null` is caused by `FormAssistant.updateSelection()`. This method will send the focused input element info to virtual keyboard. If no input element is focused, when `FormAssistant.updateSelection()` calls `FormAssistant.getSelectionInfo()`, we get the error.

Comment on attachment 790267 [details] [diff] [review] Fix indentation. Review of attachment 790267 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/chrome/content/forms.js @@ +536,3 @@ > target.value : > getContentEditableText(target); > Nit: I find it more readable like this: let value = isContentEditable(target) ? getContentEditableText(target) : target.value; @@ +643,2 @@ > element.value : > getContentEditableText(element); Same here. @@ +843,5 @@ > } > > // Get the visible content text of a content editable element > function getContentEditableText(element) { > + if (!element && !isContentEditable(element)) { Don't we want !element || !isContentEditable(element) ?

Fabrice, thanks. The patch addresses all the review comments and the diff with the previous one is: diff --git a/b2g/chrome/content/forms.js b/b2g/chrome/content/forms.js --- a/b2g/chrome/content/forms.js +++ b/b2g/chrome/content/forms.js @@ -527,19 +527,18 @@ let FormAssistant = { requestId: json.requestId, selectioninfo: this.getSelectionInfo() }); } break; } case "Forms:GetText": { - let value = !isContentEditable(target) ? - target.value : - getContentEditableText(target); + let value = isContentEditable(target) ? getContentEditableText(target) + : target.value; if (json.offset && json.length) { value = value.substr(json.offset, json.length); } else if (json.offset) { value = value.substr(json.offset); } @@ -634,19 +633,18 @@ let FormAssistant = { sendAsyncMessage("Forms:Input", getJSON(element, this._focusCounter)); return true; }, getSelectionInfo: function fa_getSelectionInfo() { let element = this.focusedElement; let range = getSelectionRange(element); - let text = !isContentEditable(element) ? - element.value : - getContentEditableText(element); + let text = isContentEditable(element) ? getContentEditableText(element) + : element.value; let textAround = getTextAroundCursor(text, range); let changed = this.selectionStart !== range[0] || this.selectionEnd !== range[1] || this.textBeforeCursor !== textAround.before || this.textAfterCursor !== textAround.after; @@ -839,17 +837,17 @@ function getDocumentEncoder(element) { Ci.nsIDocumentEncoder.OutputLFLineBreak | Ci.nsIDocumentEncoder.OutputNonTextContentAsPlaceholder; encoder.init(element.ownerDocument, "text/plain", flags); return encoder; } // Get the visible content text of a content editable element function getContentEditableText(element) { - if (!element && !isContentEditable(element)) { + if (!element || !isContentEditable(element)) { return null; } let doc = element.ownerDocument; let range = doc.createRange(); range.selectNodeContents(element); let encoder = FormAssistant.documentEncoder; encoder.setRange(range);

https://hg.mozilla.org/integration/b2g-inbound/rev/0d349a4ea3cf Thanks for making quick work of these, you rock!

B2G mochitest-3 is officially free of forms.js exceptions now! Unfortunately, mochitest-2 still has a bunch :( https://tbpl.mozilla.org/php/getParsedLog.php?id=26553373&tree=B2g-Inbound 13:00:40 INFO - 3917 INFO TEST-PASS | /tests/content/html/content/test/forms/test_change_event.html | Change event shouldn't be dispatched for checkbox ---> text input type change 13:00:40 INFO - System JS : ERROR chrome://browser/content/forms.js:645 13:00:40 INFO - TypeError: element is null

Sorry, I missed a check of the focused element for FormAssistant.updateSelection() within a timer callback function as following. ========================== diff --git a/b2g/chrome/content/forms.js b/b2g/chrome/content/forms.js --- a/b2g/chrome/content/forms.js +++ b/b2g/chrome/content/forms.js @@ -398,16 +398,17 @@ let FormAssistant = { } // Don't monitor the text change resulting from key event. this._ignoreEditActionOnce = true; // We use 'setTimeout' to wait until the input element accomplishes the // change in selection range. content.setTimeout(function() { + // Should make sure this.focusedElement is not null! this.updateSelection(); }.bind(this), 0); break; case "keyup": if (!this.focusedElement) { break; } ========================== The patch will check the focused element at the begining of FormAssistant.updateSelection, so that we won't have the same issue again.

https://hg.mozilla.org/mozilla-central/rev/f107713640cb B2G mochitest-2 is now squeaky clean! Thanks :)