Closed Bug 1173604 Opened 5 years ago Closed 4 years ago

Editing a css property auto-inserts autocomplete text

Categories

(DevTools :: Inspector, defect, P3)

41 Branch
x86_64
Windows 10
defect

Tracking

(firefox41 affected)

RESOLVED WORKSFORME
Tracking Status
firefox41 --- affected

People

(Reporter: verdi, Unassigned)

References

Details

(Whiteboard: [btpp-backlog])

Attachments

(2 files)

Attached video devtools-bug.mp4
I'm using the latest Windows 10 build and the latest nightly. When I try to edit a css property with the dev tools, as soon as I make the property editable, auto-complete text gets inserted. I'm attaching a video since this is difficult to explain.
Looks like a problem with the inplace editor.  Not the same problem as Bug 1032984, but could be a similar issue.  Does this happen on any property value?  Also, does it happen on property names as well?
See Also: → 1032984
(In reply to Brian Grinstead [:bgrins] from comment #1)
> Looks like a problem with the inplace editor.  Not the same problem as Bug
> 1032984, but could be a similar issue.  Does this happen on any property
> value?  Also, does it happen on property names as well?

Also, are you making it editable by clicking on it, or using the keyboard, or...?
Flags: needinfo?(mverdi)
It seems it happens with both the names and the values and it happens if I make them editable both by clicking or by using the keyboard.
Flags: needinfo?(mverdi)
I can't reproduce this on my surface pro on win10 build 10130 (which I believe is the latest). Both clicking and keyboard-focusing works fine.

Is this a clean profile? Tried in safe mode? Anything else that might be particular about the setup? (I don't really see how this is could be related to win10 at all so far...)
Flags: needinfo?(mverdi)
(In reply to :Gijs Kruitbosch from comment #4)
> I can't reproduce this on my surface pro on win10 build 10130 (which I
> believe is the latest). Both clicking and keyboard-focusing works fine.
> 
> Is this a clean profile? Tried in safe mode? Anything else that might be
> particular about the setup? (I don't really see how this is could be related
> to win10 at all so far...)

This happens in safe mode and with a new profile. I also tried it without my external keyboard and mouse and it still happens.
Flags: needinfo?(mverdi)
(In reply to Verdi [:verdi] from comment #5)
> (In reply to :Gijs Kruitbosch from comment #4)
> > I can't reproduce this on my surface pro on win10 build 10130 (which I
> > believe is the latest). Both clicking and keyboard-focusing works fine.
> > 
> > Is this a clean profile? Tried in safe mode? Anything else that might be
> > particular about the setup? (I don't really see how this is could be related
> > to win10 at all so far...)
> 
> This happens in safe mode and with a new profile. I also tried it without my
> external keyboard and mouse and it still happens.

OK. Which win10 build is this, and which website are you using to test? Can you write down precise steps from the moment of opening Firefox?
Flags: needinfo?(mverdi)
Attached image devtools.jpg
I'm using the latest Windows 10 build 10130. I'm using Nightly 41.0a1, Build ID 20150611030208 and a fresh profile.

1. Right click on the blue Save Changes button below the text input field on this page and choose Inspect element.
2. On the right side you should see the Rules tab. The first element should be button. Click the button color (#43A6E2) to make it editable and then watch as auto-complete values get auto inserted. In this case I saw "aliceblue" being inserted over and over again.
3. Hit ESC to stop.
Flags: needinfo?(mverdi)
(In reply to Verdi [:verdi] from comment #7)
> Created attachment 8621560 [details]
> devtools.jpg
> 
> I'm using the latest Windows 10 build 10130. I'm using Nightly 41.0a1, Build
> ID 20150611030208 and a fresh profile.
> 
> 1. Right click on the blue Save Changes button below the text input field on
> this page and choose Inspect element.
> 2. On the right side you should see the Rules tab. The first element should
> be button. Click the button color (#43A6E2) to make it editable and then
> watch as auto-complete values get auto inserted. In this case I saw
> "aliceblue" being inserted over and over again.
> 3. Hit ESC to stop.

I haven't been able to reproduce on any OS / device, but I totally believe you!  Do you have any pointing devices or keyboards plugged in or connected via bluetooth?

If you open up the Browser Console with ctrl+shift+j do you see any errors or warnings?
(In reply to Brian Grinstead [:bgrins] from comment #8) 
> I haven't been able to reproduce on any OS / device, but I totally believe
> you!  

Me either! :(

Do you have any pointing devices or keyboards plugged in or connected
> via bluetooth?

I don't think I have anything hooked up by bluetooth. I have a logitech wireless mouse that uses a dongle - thought those didn't use bluetooth. Regardless, the problem still happens if I turn the mouse off and remove the dongle.

> 
> If you open up the Browser Console with ctrl+shift+j do you see any errors
> or warnings?

Yes I get a JS error:
10:06:26.759 TypeError: this._list.ensureIndexIsVisible is not a function1 autocomplete-popup.js:305:4
(In reply to Verdi [:verdi] from comment #9)
> > If you open up the Browser Console with ctrl+shift+j do you see any errors
> > or warnings?
> 
> Yes I get a JS error:
> 10:06:26.759 TypeError: this._list.ensureIndexIsVisible is not a function1
> autocomplete-popup.js:305:4

Oh, that looks *very* relevant.

That popup is used in the rule view: https://github.com/mozilla/gecko-dev/blob/f5cca86bcb4f4a2f263225903a25f9cb64b47519/browser/devtools/styleinspector/rule-view.js#L1222 and passed into the inplace editors when editing properties: https://github.com/mozilla/gecko-dev/blob/f5cca86bcb4f4a2f263225903a25f9cb64b47519/browser/devtools/styleinspector/rule-view.js#L3197.

It looks like there is at least one place we are already checking that ensureIndexIsVisible exists before calling it: https://github.com/mozilla/gecko-dev/blob/f5cca86bcb4f4a2f263225903a25f9cb64b47519/browser/devtools/shared/autocomplete-popup.js#L367.

I have a hunch about it.. let me put together a try push
I don't know why ensureIndexIsVisible wouldn't exist, but there was a check added here: https://hg.mozilla.org/mozilla-central/rev/ca621a4ceaa1#l18.1 for Bug 820524
Here is a try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7fff0849d60.  Sometime after the build for Windows finishes (~1-2 hours), can you grab the binary and see if it still has the problem locally?
Flags: needinfo?(mverdi)
(In reply to Brian Grinstead [:bgrins] from comment #13)
> Alright, the try push is built for windows:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bgrinstead@mozilla.
> com-b7fff0849d60/try-win32/

I tried this and the same thing happened but I got a different JS error:
14:36:27.671 Key event not available on GTK2: key="u" modifiers="shift, accel"1 toolbox.xul
Flags: needinfo?(mverdi)
That new error isn't related.

Let's try and narrow this down further. I have a tentative hypothesis regarding our recent windows touch events work... to test that:

Is this a recent regression for you (quick test: does it happen on 38 release / 39 beta) ? And am I right to assume you're on a laptop? Does it have a touchscreen and/or touchpad?
Flags: needinfo?(mverdi)
(In reply to :Gijs Kruitbosch from comment #15)
> 
> Is this a recent regression for you (quick test: does it happen on 38
> release / 39 beta) ? 

The newest version I can find that works as expected is 29.0.1. I tested 30.0, 31.1.1, 35.0.1, and 38.0.5 and they all have the problem.

And am I right to assume you're on a laptop? Does it
> have a touchscreen and/or touchpad?

Yes.
Flags: needinfo?(mverdi)
(In reply to Verdi [:verdi] from comment #16)
> (In reply to :Gijs Kruitbosch from comment #15)
> > 
> > Is this a recent regression for you (quick test: does it happen on 38
> > release / 39 beta) ? 
> 
> The newest version I can find that works as expected is 29.0.1. I tested
> 30.0, 31.1.1, 35.0.1, and 38.0.5 and they all have the problem.

Huh, OK. Not what I expected, but any regression range here would be better than what we have now. Any chance you could use mozregression ( http://mozilla.github.io/mozregression/ ) to narrow this down to a 1-day m-c regression window?
Flags: needinfo?(mverdi)
(In reply to :Gijs Kruitbosch from comment #17)
 Any chance you could use mozregression (
> http://mozilla.github.io/mozregression/ ) to narrow this down to a 1-day m-c
> regression window?

I'll give it try - never used mozregression before.
(In reply to :Gijs Kruitbosch from comment #17)
Any chance you could use mozregression (
> http://mozilla.github.io/mozregression/ ) to narrow this down to a 1-day m-c
> regression window?

Here's what I got:

3:26.79 LOG: MainThread Bisector INFO Narrowed nightly regression window from [
2014-03-25, 2014-03-27] (2 days) to [2014-03-25, 2014-03-26] (1 days) (~0 steps
left)
 3:26.79 LOG: MainThread Bisector INFO Got as far as we can go bisecting nightli
es...
 3:26.80 LOG: MainThread Bisector INFO Last good revision: 5c0673441fc8
 3:26.80 LOG: MainThread Bisector INFO First bad revision: 140ac04d7454
 3:26.80 LOG: MainThread Bisector INFO Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5c0673441fc8&tocha
nge=140ac04d7454

 3:26.80 LOG: MainThread Bisector INFO ... attempting to bisect inbound builds (
starting from 4 days ago, to make sure no inbound revision is missed)
 3:27.59 LOG: MainThread Bisector INFO Getting inbound builds between c148f0b0c8
b4 and 140ac04d7454
C:\Python27\lib\site-packages\requests\packages\urllib3\util\ssl_.py:90: Insecur
ePlatformWarning: A true SSLContext object is not available. This prevents urlli
b3 from configuring SSL appropriately and may cause certain SSL connections to f
ail. For more information, see https://urllib3.readthedocs.org/en/latest/securit
y.html#insecureplatformwarning.
  InsecurePlatformWarning
 3:37.49 LOG: MainThread Bisector INFO No inbound data found.
 3:37.49 LOG: MainThread Bisector INFO There are no build dirs on inbound for th
ese changesets.
Flags: needinfo?(mverdi)
Looks like one of these...

	c354ec771e52	Girish Sharma — Bug 913955 - Avoid race conditions and inserting in middle of text in inplace-editor, r=mratcliffe
	7621dd6d9091	Girish Sharma — Bug 912189 - Show CSS value autocomplete without waiting for first character, r=mratcliffe
	a4c4616c884a	Girish Sharma — Bug 971702 - Style editor to provide meaningful information about hovered text, r=pbrosset


Mike, any ideas what's going on here?
Flags: needinfo?(mratcliffe)
(In reply to :Gijs Kruitbosch from comment #20)
> Looks like one of these...
> 
> 	c354ec771e52	Girish Sharma — Bug 913955 - Avoid race conditions and
> inserting in middle of text in inplace-editor, r=mratcliffe
> 	7621dd6d9091	Girish Sharma — Bug 912189 - Show CSS value autocomplete
> without waiting for first character, r=mratcliffe
> 	a4c4616c884a	Girish Sharma — Bug 971702 - Style editor to provide
> meaningful information about hovered text, r=pbrosset
> 
> 
> Mike, any ideas what's going on here?

I can't reproduce it either but I suspect graphics drivers. To check this it would be great if @verdi could go to Preferences -> Advanced -> General and uncheck "Use hardware acceleration when available." Restart the browser and let us know if the problem still exists (revert the setting after checking). Although, that would only give us a root cause, which is not very useful in itself.

Drivers and bug numbers aside we really need to add logging to see why browser/devtools/shared/inplace-editor.js::_maybeSuggestCompletion() is showing the autocomplete popup in this situation. It is a shame that we don't have anybody that can reproduce this so that we can investigate it properly.

I don't have time at the moment as I am 100% focused on the Storage Inspector but I can guarantee that if you add a lot of logging to that method it will be clear why we are showing autocomplete during each timeout loop.
Flags: needinfo?(mratcliffe)
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #21)

> 
> I can't reproduce it either but I suspect graphics drivers. To check this it
> would be great if @verdi could go to Preferences -> Advanced -> General and
> uncheck "Use hardware acceleration when available." Restart the browser and
> let us know if the problem still exists (revert the setting after checking).
> Although, that would only give us a root cause, which is not very useful in
> itself.

The problem still exists with hardware acceleration turned off.
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #21)
> Drivers and bug numbers aside we really need to add logging to see why
> browser/devtools/shared/inplace-editor.js::_maybeSuggestCompletion() is
> showing the autocomplete popup in this situation. It is a shame that we
> don't have anybody that can reproduce this so that we can investigate it
> properly.
> 
> I don't have time at the moment as I am 100% focused on the Storage
> Inspector but I can guarantee that if you add a lot of logging to that
> method it will be clear why we are showing autocomplete during each timeout
> loop.

We could generate a try push that has necessary logging / tracing and ask :verdi to run through it and paste in the results from the Browser Console.
Jared thinks this isn't specific to win10, so we're going to take it off the win10 tracker.
No longer blocks: windows-10
(In reply to Brian Grinstead [:bgrins] from comment #23)
> (In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #21)
> > Drivers and bug numbers aside we really need to add logging to see why
> > browser/devtools/shared/inplace-editor.js::_maybeSuggestCompletion() is
> > showing the autocomplete popup in this situation. It is a shame that we
> > don't have anybody that can reproduce this so that we can investigate it
> > properly.
> > 
> > I don't have time at the moment as I am 100% focused on the Storage
> > Inspector but I can guarantee that if you add a lot of logging to that
> > method it will be clear why we are showing autocomplete during each timeout
> > loop.
> 
> We could generate a try push that has necessary logging / tracing and ask
> :verdi to run through it and paste in the results from the Browser Console.

To be clear, I'm asking if you could generate said push when you have a chance.  I'd be happy to help - but I'm not sure exactly what should be logged and where, and I'm guessing it's just as easy to put together a patch as it is to explain where to put things.
Flags: needinfo?(mratcliffe)
Verdi: Could you check if the bug still occurs for you?

Bug inspector triage. Filter on CLIMBING SHOES.
P3 for now.
Flags: needinfo?(mverdi)
Priority: -- → P3
Whiteboard: [btpp-backlog]
I can no longer reproduce this.
Flags: needinfo?(mverdi)
Thanks for checking! Closing as worksforme.
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(mratcliffe)
Resolution: --- → WORKSFORME
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.