Switch to Tab no more 0.1 breaks awesomebar since 20120923 Nightly

VERIFIED FIXED in Firefox 18

Status

()

VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: ehoogeveen, Assigned: jaws)

Tracking

({regression})

Trunk
mozilla18
regression
Points:
---

Firefox Tracking Flags

(firefox18+ verified)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
Since the 20120923 Nightly my awesomebar has been broken. Thanks to some digging by Pr0phet[1] we found that "Switch to Tab no more 0.1"[2] was responsible for the issues. This might be INVALID if the this is an intentional change, but I figured I'd file it anyway. Looking at the changelog for yesterday's Nightly, I think bug 781588 might be responsible.

Expected results:
Awesomebar continues to work as expected, with the "Switch to tab" option removed from the dropdown history.

Actual results:
Dropdown history does not show up until the arrow keys are pressed several times, and does not *update* until text is cleared and typing begins anew. In addition, pasting text into the awesomebar is broken.

[1] http://forums.mozillazine.org/viewtopic.php?p=12317369#p12317369
[2] https://addons.mozilla.org/en-US/firefox/addon/switch-to-tab-no-more/

Comment 1

7 years ago
Emanuel,

Maybe ask at http://forums.mozillazine.org/viewtopic.php?f=38&t=2287733 which is listed as "Support site" for "Switch to Tab no more".
I just installed "Switch to Tab no more" on the 9-24 Nightly and I'm not seeing any issues with the Awesome Bar.

Can you include some better steps to reproduce and reopen?
Status: UNCONFIRMED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → INVALID
(Reporter)

Comment 3

7 years ago
Interesting, I can confirm that the problem happens with all other extensions disabled, so there might be some other interaction with my profile. Will do more testing in the morning.
(Reporter)

Comment 4

7 years ago
Okay, for me the following steps consistently reproduce the behavior I reported above:

1) From a clean profile, navigate to https://addons.mozilla.org/en-US/firefox/addon/switch-to-tab-no-more/ and install the extension.
2) Restart the browser to finish the installation. At this point, pasting things into the address bar should be broken.
3) Navigate to http://twitch.tv/team/srl and restart the browser.
4) Type the letter 's' into the address bar. No history dropdown should appear until the arrow keys are used (and what does appear isn't dynamic, i.e. it doesn't depend on what you type afterward until you clear the text and start over with the dropdown still open).

Hopefully that will give you enough info to reproduce.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Can you disable browser.urlbar.formatting.enabled in about:config and try again? The code that I changed will be disabled if that pref is disabled.
(Reporter)

Comment 6

7 years ago
browser.urlbar.formatting.enabled
https://addons.mozilla.org/en-US/firefox/addon/switch-to-tab-no-more/

Apologies for the delayed response, but I found something curious in testing and needed a bit more free time to confirm. As it turns out, using the steps I listed in comment #4, the behavior I mentioned disappears after restarting the browser once more. Pasting works again, and so does the history dropdown. As this is hardly the case for my regular profile, it seems I don't know how to get it into the broken state permanently.

However, changing the setting you mentioned *prior* to installing the extension, I did notice a change in behavior:
1) From a clean profile, go to about:config and set browser.urlbar.formatting.enabled to false, then restart the browser.
2) Navigate to https://addons.mozilla.org/en-US/firefox/addon/switch-to-tab-no-more/ and install the extension.
3) Restart the browser to finish the installation. At this point, pasting things into the address bar should be broken.
4) Navigate to http://twitch.tv/team/srl and restart the browser.
5) Type the letter 's' into the address bar. Unlike before, the history dropdown should appear as normal. However, pasting things into the address bar is still broken.

Restarting the browser again after this point will restore both the history dropdown and pasting behavior to normal regardless of browser.urlbar.formatting.enabled. So the behavior is a bit.. elusive, at least with the STR I used here. Hopefully the difference I found will still give you a clue as to what could be going on.
(Reporter)

Comment 7

7 years ago
Ah, those two lines at the top of my comment were left over from my notepad as I was testing and writing the comment.

Comment 8

7 years ago
I had the same issue too, though I have no reliable repro steps. It seems that a well populated Places database is required for this bug to manifest.

The browser.urlbar.formatting.enabled pref doesn't seem to have any effect.
(Reporter)

Comment 9

7 years ago
Well, I was able to confirm that bug 781588 caused this regression:

Last good (local build): 
https://hg.mozilla.org/integration/mozilla-inbound/rev/9108fa673bfe

First bad (local build):
https://hg.mozilla.org/integration/mozilla-inbound/rev/f753afbe0d85

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9108fa673bfe&tochange=f753afbe0d85

If my STR aren't enough to allow you to debug this, could I e-mail you the relevant parts of my profile?
Do you see any errors in your Error Console? And if you set browser.urlbar.formatting.enabled to false then the problem goes away?

Comment 11

6 years ago
Negative to both questions.
browser.urlbar.formatting.enabled=false completely disables the code changed by bug 781588. Can you try with a new profile?

Comment 13

6 years ago
Reproducing the issue in a new profile with a manually copied, populated places database is rare (though it does happen some times) and I couldn't correlate the pref state with the issue happening :( .

The good news it that using my normal profile, where the issue is reliably reproducible, the state of the pref *does* make a difference in the bug appearing this time. I don't remember if I restarted the browser last time I tried the pref so sorry for any confusion.

Will give it another try tomorrow

Comment 14

6 years ago
The following entries spam the Error Console.

Timestamp: 07/10/2012 01:01:31
Error: TypeError: this.editor is null
Source File: chrome://browser/content/urlbarBindings.xml
Line: 159

Timestamp: 07/10/2012 01:03:54
Error: TypeError: this.editor is null
Source File: chrome://browser/content/urlbarBindings.xml
Line: 216
(Reporter)

Comment 15

6 years ago
I ran some tests with debugging builds of the above changesets and logged the stderr output to text files. In all cases, there were no differences when comparing
1) [lastgood without addon] vs [lastgood with addon]
2) [lastgood without addon] vs [firstbad without addon]

In all cases, there were differences when comparing [firstbad without addon] vs [firstbad with addon]. I didn't test with browser.urlbar.formatting.enabled set to false, let me know if I should.

----------------------

1st case: 
1) Restore session with only about:addons open
2) Wait 30 seconds
3) Exit

Error lines in [no addon] but not in [with addon]:
WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:/Users/firefox/netwerk/base/src/../../../../netwerk/base/src/nsSimpleURI.cpp, line 273
WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:/Users/firefox/netwerk/base/src/../../../../netwerk/base/src/nsSimpleURI.cpp, line 273

Error lines in [with addon] but not in [no addon]:

----------------------

2nd case:
1) Restore session with only about:home open and the text 'srl' in the address bar
2) Wait 30 seconds
3) Select text in address bar, type in 'srl' (so history dropdown appears)
4) Wait 30 seconds
5) Exit

Error lines in [no addon] but not in [with addon]:
WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:/Users/firefox/netwerk/base/src/../../../../netwerk/base/src/nsSimpleURI.cpp, line 273
WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:/Users/firefox/netwerk/base/src/../../../../netwerk/base/src/nsSimpleURI.cpp, line 273
WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:/Users/firefox/netwerk/base/src/../../../../netwerk/base/src/nsSimpleURI.cpp, line 273
WARNING: Nv3DVStreaming CoCreateInstance failed (disabled).: file c:/Users/firefox/gfx/layers/../../../gfx/layers/d3d9/Nv3DVUtils.cpp, line 53

Error lines in [with addon] but not in [no addon]:
JavaScript error: chrome://browser/content/urlbarBindings.xml, line 216: this.editor is null
JavaScript error: chrome://browser/content/urlbarBindings.xml, line 159: this.editor is null

----------------------

3rd case:
1) Restore session with only about:home open no text in the address bar
2) Wait 30 seconds
3) Paste the text 'paste' into the address bar
4) Wait 30 seconds
5) Exit

Error lines in [no addon] but not in [with addon]:
WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:/Users/firefox/netwerk/base/src/../../../../netwerk/base/src/nsSimpleURI.cpp, line 273
WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:/Users/firefox/netwerk/base/src/../../../../netwerk/base/src/nsSimpleURI.cpp, line 273
WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:/Users/firefox/netwerk/base/src/../../../../netwerk/base/src/nsSimpleURI.cpp, line 273
WARNING: Nv3DVStreaming CoCreateInstance failed (disabled).: file c:/Users/firefox/gfx/layers/../../../gfx/layers/d3d9/Nv3DVUtils.cpp, line 53

Error lines in [with addon] but not in [no addon]:
JavaScript error: chrome://browser/content/urlbarBindings.xml, line 216: this.editor is null
JavaScript error: chrome://global/content/bindings/textbox.xml, line 156: this.editor is null
JavaScript error: chrome://global/content/bindings/autocomplete.xml, line 542: this._autocomplete.editor is null
JavaScript error: chrome://browser/content/urlbarBindings.xml, line 159: this.editor is null
(Reporter)

Updated

6 years ago
Blocks: 781588
Created attachment 668942 [details] [diff] [review]
Patch

It appears that the changing of textbox.editor to a field is causing issues for some users as the value is being cached before it is useful (hence being null for the full session).

Ehsan, do you know any ways to guarantee that editor is non-null while still using a field? The perf hit from using a property was quite noticeable, but correctness is obviously more important.
Assignee: nobody → jaws
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #668942 - Flags: review?(ehsan)

Comment 17

6 years ago
So this is the overlay that this add-on loads:

http://pastebin.mozilla.org/1860262

I don't see anything there which would cause this kind of problem.  Jared, have you tried setting a breakpoint in nsHTMLInputElement::GetEditor and see why it's returning null?  This should not really happen, and this patch may just wallpaper over a real bug.

Also, why does textbox.xml implement the inputField member as a property on top of a field?  <http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/textbox.xml#51>  I assume there's a good reason for that.  Perhaps that's what needs to be done here?
Component: Extension Compatibility → XUL Widgets
Product: Firefox → Toolkit
Created attachment 668955 [details] [diff] [review]
Patch v2

It's returning null because it stops at this point in nsTextEditorState::PrepareEditor,
> if (!mBoundFrame) {
>   // Cannot create an editor without a bound frame.
>   // Don't return a failure code, because js callers can't handle that.
>   return NS_OK;
> }

mBoundFrame is null when this is called. This is the call stack, http://pastebin.mozilla.org/1860315

I looked in the CVS history for the reasoning behind the property-on-top-of-field for inputField, but all I ended up with was https://github.com/jrmuizel/mozilla-cvs-history/commit/95c14745fea088c5b53165901de3acca2a00f6d5.

I switched the patch to use this solution since it will have better perf once this.editor is non-null. This is the callstack when the mBoundFrame is non-null: http://pastebin.mozilla.org/1860321.
Attachment #668942 - Attachment is obsolete: true
Attachment #668942 - Flags: review?(ehsan)
Attachment #668955 - Flags: review?(ehsan)

Comment 19

6 years ago
Comment on attachment 668955 [details] [diff] [review]
Patch v2

Ah, ok, that makes sense.  We can't create an editor before a frame has been constructed for the input element.  This will correctly handle that case.
Attachment #668955 - Flags: review?(ehsan) → review+

Updated

6 years ago
tracking-firefox18: --- → ?
Keywords: regression

Comment 20

6 years ago
(This can break more than that add-on.)
Created attachment 668956 [details] [diff] [review]
Patch for checkin (added r=ehsan)
Attachment #668955 - Attachment is obsolete: true
Comment on attachment 668956 [details] [diff] [review]
Patch for checkin (added r=ehsan)

>       <field name="mInputField">null</field>
>       <field name="mIgnoreClick">false</field>
>       <field name="mIgnoreFocus">false</field>
>+      <field name="mEditor">false</field>
[I would have used null for consistency with mInputField which is an object rather than a boolean.]
https://hg.mozilla.org/mozilla-central/rev/bf0aa600323f
https://hg.mozilla.org/mozilla-central/rev/2ec21afa445b
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
(Reporter)

Comment 26

6 years ago
In today's nightly all the broken behaviors are gone. I also repeated the test from comment #15 with local debugging builds using the changesets just before and after the fix, and the errors are all gone - the log files with and without the addon once again match up. Thanks for the fix!
Status: RESOLVED → VERIFIED
Please nominate for uplift to Aurora 18 as soon as possible. Thanks!
tracking-firefox18: ? → +
No need to track this, it got fixed before the merge so it's already in Aurora 18 :)
tracking-firefox18: + → ---

Comment 29

6 years ago
(In reply to Jared Wein [:jaws] from comment #28)
> No need to track this, it got fixed before the merge so it's already in
> Aurora 18 :)

Nope, we still need to track it to make sure that it will reappear on radar if the patch gets backed out from 18 or something.  :-)
status-firefox18: --- → fixed
tracking-firefox18: --- → +
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:18.0) Gecko/18.0 Firefox/18.0 beta 6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20130102 Firefox/20.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:18.0) Gecko/18.0 Firefox/18.0 beta 6
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:20.0) Gecko/20130103 Firefox/20.0
Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/18.0 Firefox/18.0 beta 6
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20130103 Firefox/20.0
`
Verified as fixed on latest nightly and beta on above builds.
status-firefox18: fixed → verified
You need to log in before you can comment on or make changes to this bug.