Closed
Bug 793715
Opened 12 years ago
Closed 12 years ago
Switch to Tab no more 0.1 breaks awesomebar since 20120923 Nightly
Categories
(Toolkit :: XUL Widgets, defect)
Toolkit
XUL Widgets
Tracking
()
VERIFIED
FIXED
mozilla18
People
(Reporter: ehoogeveen, Assigned: jaws)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
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•12 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".
Assignee | ||
Comment 2•12 years ago
|
||
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
Closed: 12 years ago
Resolution: --- → INVALID
Reporter | ||
Comment 3•12 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•12 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 → ---
Assignee | ||
Comment 5•12 years ago
|
||
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•12 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•12 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.
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•12 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?
Assignee | ||
Comment 10•12 years ago
|
||
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•12 years ago
|
||
Negative to both questions.
Assignee | ||
Comment 12•12 years ago
|
||
browser.urlbar.formatting.enabled=false completely disables the code changed by bug 781588. Can you try with a new profile?
Comment 13•12 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•12 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•12 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
Assignee | ||
Comment 16•12 years ago
|
||
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•12 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?
Updated•12 years ago
|
Component: Extension Compatibility → XUL Widgets
Product: Firefox → Toolkit
Assignee | ||
Comment 18•12 years ago
|
||
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•12 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•12 years ago
|
tracking-firefox18:
--- → ?
Keywords: regression
Comment 20•12 years ago
|
||
(This can break more than that add-on.)
Assignee | ||
Comment 21•12 years ago
|
||
Attachment #668955 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 22•12 years ago
|
||
Keywords: checkin-needed
Comment 23•12 years ago
|
||
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.]
Assignee | ||
Comment 24•12 years ago
|
||
Comment 25•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bf0aa600323f
https://hg.mozilla.org/mozilla-central/rev/2ec21afa445b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Reporter | ||
Comment 26•12 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
Assignee | ||
Comment 28•12 years ago
|
||
No need to track this, it got fixed before the merge so it's already in Aurora 18 :)
tracking-firefox18:
+ → ---
Comment 29•12 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:
--- → +
Comment 30•12 years ago
|
||
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.
Updated•12 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•