1209149 - After page reload, "Highlight painted area" is inactive, but the button has pressed state

Status: RESOLVED DUPLICATE of bug 1342928

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect, P3)

Description

[arni2033]

STR:   (Win7_64, Nightly 44, 32bit, ID 20150926030216, new profile, safe mode)
1. Open the following "data:" url of click URL in the form above
>   data:text/html,<input value="asdf">
2. Open devtools, make sure that you have "Highlight painted area" button on devtools toolbar
3. Press that button
4. Move mouse over <input> to make sure that highlighted area is being painted
5. Reload the page
6. Move mouse over <input> to make sure that highlighted area isn't being painted
7. Click "Highlight painted area" button

Results:      After Step 5 that "Highlight painted area" feature "switches off" (OK)
              But the button still has "Pressed" state while that function isn't actually activated
              After Step 7 the button doesn't change visually, but the feature "switches on"

Expectations: The button should always reflect current state of the "Highlight painted area" feature

Comment 1

[arni2033]

This is regression from bug 1128988 (between 2015-04-23 and 2015-04-24). Regression range:
> https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0b202671c9e2&tochange=22a157f7feb7

Before bug 1128988 this feature didn't work in e10s mode, but in non-e10s mode it worked correctly.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Patrick, do you know who could help with this regression? Thanks

Comment 3

[Jarda Snajdr [:jsnajdr]]

I can have a look at it, if it's considered important and worth uplifting to aurora or even beta.

Comment 4

[Patrick Brosset <:pbro>]

(In reply to Jarda Snajdr [:jsnajdr] from comment #3)
> I can have a look at it, if it's considered important and worth uplifting to
> aurora or even beta.
Thanks Jarda for volunteering for this. 
We've had similar problems in several occasions in the past for other command buttons, like the ruler or measurement tool buttons. I remember Matteo had fixed this by having some WeakMap of the window IDs where the tool was activated, or something like this.
The code in this command file should help I think: devtools\shared\gcli\commands\rulers.js
Otherwise Matteo (:zer0) can help as well.

Note that this isn't the most pressing issue either. It only occurs on page reload. Yes it is confusing, but it's not a very often used button.
If you're working on something else at the moment, this bug is probably less important than that.

Comment 5

[arni2033]

(In reply to Patrick Brosset <:pbro> from comment #4)
> It only occurs on page reload.
You're wrong.

Comment 6

[Patrick Brosset <:pbro>]

(In reply to arni2033 from comment #5)
> (In reply to Patrick Brosset <:pbro> from comment #4)
> > It only occurs on page reload.
> You're wrong.
Could you elaborate please? I thought the problem described in this bug was where after the page was reloaded (step 5 in the STR), the button stayed active.

Comment 7

[arni2033]

I was too busy to post the sensible part.
Also, by saying that (comment 5) I didn't mean that it's a serious bug. I just don't like it when people try to make bugs look more insignificant.


>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
STR_2:
1. Open this bug (bug 1209149)
2. Open devtools, click "Highlight painted area" button on devtools toolbar
3. Execute  "location.href='show_bug.cgi?id=1268736'"  in console (w/o outer quotes)
4. Hover mouse over any <select> on the page to see if "Highlight painted area" is enabled

AR:  Button looks enabled. Highlight is disabled
ER:  Button should match highlight state


STR_3:
1. Open this bug (bug 1209149)
2. Open devtools, click "Highlight painted area" button on devtools toolbar
3. Execute  "location.href='show_bug.cgi?id=1268736'"  in console (w/o outer quotes)
4. Click "Highlight painted area" button. Click "Highlight painted area" button.
5. Execute  "history.go(-1)"  in console (w/o outer quotes)
6. Hover mouse over any <select> on the page to see if "Highlight painted area" is enabled

AR:  Button looks disabled. Highlight is enabled
ER:  Button should match highlight state


Notes:
1) Steps 3 and 5 are equivalent to normal user actions in urlbar
2) I haven't checked regression ranges for STR_2 and STR_3

Comment 8

[Patrick Brosset <:pbro>]

Mike, I'm setting P3, but ultimately this is your component to triage, so feel free to change it.
P3 because this is a hidden-by-default tool.

Jarda, you said in comment 3 that you might interested in fixing this. Is it still the case?

Matteo, see comment 4. Could you please provide links to other parts of the code where this was fixed already?

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

P3 sounds right to me.

Comment 10

[Matteo Ferretti [:zer0] [:matteo]]

(In reply to Patrick Brosset <:pbro> from comment #8)

> Matteo, see comment 4. Could you please provide links to other parts of the
> code where this was fixed already?

Part of the code is already there (`isCheckedFor`): 
https://dxr.mozilla.org/mozilla-central/source/devtools/shared/gcli/commands/paintflashing.js#27-28

What is missing, it's something similar to: 

https://dxr.mozilla.org/mozilla-central/source/devtools/shared/gcli/commands/rulers.js#96-104

That basically takes care of when the highlighter is destroyed, in order to clean up the set. Maybe it could be just adding a `enabledPaintFlashing.delete(id)` in the target's navigate event.

Comment 11

[Jarda Snajdr [:jsnajdr]]

(In reply to Patrick Brosset <:pbro> from comment #8)
> Jarda, you said in comment 3 that you might interested in fixing this. Is it
> still the case?

Still on my radar, but I'm too busy with Netmonitor these days.

Comment 12

[Matteo Ferretti [:zer0] [:matteo]]

(In reply to Jarda Snajdr [:jsnajdr] from comment #11)
> (In reply to Patrick Brosset <:pbro> from comment #8)
> > Jarda, you said in comment 3 that you might interested in fixing this. Is it
> > still the case?
> 
> Still on my radar, but I'm too busy with Netmonitor these days.

Jarda, if you're too busy with Netmonitor no worries; I can take it. Let me know!

Comment 13

[Jarda Snajdr [:jsnajdr]]

(In reply to Matteo Ferretti [:zer0] [:matteo] from comment #12)
> Jarda, if you're too busy with Netmonitor no worries; I can take it. Let me know!

Yes Matteo, please take it. I'm not going to get to this anytime soon.

Comment 14

[Matteo Ferretti [:zer0] [:matteo]]

I believe this bug was fixed by Bug 1342928. Feel free to reopen if that is not the case.