When I try to log into GitHub from an e10s window, I see the following JS error in Firefox's Browser Console (not the page's Web Console): > TypeError: this.mInput is null autocomplete.xml:898 This error does not happen in a non-e10s window.
Created attachment 8444787 [details] [diff] [review] The attached changes to file autocomplete.xml fix the bug. Basically, all I do is null-check mInput or the future value of mInput.
Comment on attachment 8444787 [details] [diff] [review] The attached changes to file autocomplete.xml fix the bug. hi Alexandru, to request a code review, you should attach a patch, not the changed file, and then set the review flag to "?" and enter an appropriate reviewer's name. This MDN article is a good introduction to creating a patch and requesting a code review: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
Created attachment 8445217 [details] [diff] [review] Fixed the bug by null-checking mInput whenever it is used. r=felipe
I'll get to this review soon.. I had a note about this .mInput being null when I worked on the select dropdowns bug. I need to find it to see what it was about :) (i.e., if there's something special to consider here than just doing a null check)
Comment on attachment 8445217 [details] [diff] [review] Fixed the bug by null-checking mInput whenever it is used. r=felipe Apologies for the delay.. On my notes I found that I left the warning of .mInput being null to remind us to look into it more. Basically there are some things in autocomplete.xml that depends on properties of mInput, so we should check if those are missing features or not, before The 4 things I see in autocomplete.xml that look relevant are - showCommentColumn - showImageColumn I think those are not part of the web spec (only used in our UI code). With a double check to this, I think we can safely ignore those for out-of-process content and it should remain working in our UI (e.g. the searchbar) - maxRows this might be part of some html5 attribute that is used to customize this value - controller This looks like the hardest part. There are two different controllers at play, one is mController which is created in the constructor (and is not related to mInput), and the other is mInput.controller. A lot of things are accessed directly or indirectly through the controller. I suspect most things are already properly working, but I think what should be the goal for this bug is to investigate these 4 things and see what is missing, and what can be safely ignored and wrapped with a null-check.
Created attachment 8468922 [details] [diff] [review] Bug 1017914 - Patch v2: Null-checked mInput only in |popupshowing| and |popuphiding|. r=felipe I have investigated the issues that you had suggested. - this.mInput is used to assign showCommentColumn and showImageColumn new values in | _openAutocompletePopup |, but that method is never called in the child where mInput is null; - if this.mInput is null, then the default value for maxRows is used (which is this.defaultMaxRows); - this.mInput.controller is used only in _appendCurrentResult, view and _matchCount. As far as I can tell, neither of them is called in the child where mInput is null. They are only called in the parent (e.g. when autocompleting the URL bar). For these reasons, I think that null-checking just in popupshowing and popuphiding shuold be enough. I hope I didn't overlook anything.