807274 - [markup view] Clicking on a node doesn't lock the highlighter

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Dirkjan Ochtman (:djc)]

This happens in 18.0a2 (2012-10-28). The Style pane still shows element content if you click an element in the browser part of the content area, so something about the inspector is broken.

Comment 1

[Paul Rouget [:paul]]

This is not very clear. Can you share some STR?

Comment 2

[Dirkjan Ochtman (:djc)]

1. CTRL + Shift + I to open the Inspector
2. Click Style button on the lower right (Style pane appears, "No element selected")
3. In the DOM tree, click an element node: Style pane stays at "No element selected"

Desired result: when click an element node in the DOM tree, the same thing should happen as when I click a node in the main browser area or the breadcrumbs at the bottom of the inspector.

Comment 3

[Paul Rouget [:paul]]

Indeed.

Comment 4

[Dirkjan Ochtman (:djc)]

Can someone please look at this? It seems like a significant regression that's not too hard to fix.

Comment 5

[Paul Rouget [:paul]]

I never fixed that. And it's broken in Beta -> P1

Comment 6

[Paul Rouget [:paul]]

Attachment: patch v1

Comment 7

[Paul Rouget [:paul]]

This needs to go into beta and aurora, but not in nightly.

https://tbpl.mozilla.org/?tree=Try&rev=4f2d8a6807ee

Comment 8

[Paul Rouget [:paul]]

Comment on attachment 686545 [details] [diff] [review]
patch v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Inspector behaves strangely (every time you would click on a node in the DOM View, overring the page would change the selection)
Testing completed (on m-c, etc.): my computer and try: https://tbpl.mozilla.org/?tree=Try&rev=4f2d8a6807ee (we don't want it in m-c)
Risk to taking this patch (and alternatives if risky): medium (not heavily tested)
String or UUID changes made by this patch: no

Comment 9

[Alex Keybl [:akeybl]]

(In reply to Paul Rouget [:paul] from comment #8)
> Risk to taking this patch (and alternatives if risky): medium (not heavily
> tested)

This hasn't been on our tracking radar, and we're approaching our fourth beta. Does this have the possibility of causing a new regression that, if shipped in FF18, you would advocate a re-spin for?

Who typically performs devtools QA for you all, to take a look at the change in Aurora and subsequently Beta?

Comment 10

[Paul Rouget [:paul]]

(In reply to Alex Keybl [:akeybl] from comment #9)
> (In reply to Paul Rouget [:paul] from comment #8)
> > Risk to taking this patch (and alternatives if risky): medium (not heavily
> > tested)
> 
> This hasn't been on our tracking radar, and we're approaching our fourth
> beta. Does this have the possibility of causing a new regression that, if
> shipped in FF18, you would advocate a re-spin for?

Not very likely to cause a new regression. If any regression happen, I can't really tell what would be its nature, but surely nothing that would require a re-spin.

> Who typically performs devtools QA for you all, to take a look at the change
> in Aurora and subsequently Beta?

Afaik, nobody. If we get the approval for beta, I will make sure that someone will be actively working on testing the affected features in beta and aurora.

Comment 11

[Alex Keybl [:akeybl]]

Comment on attachment 686545 [details] [diff] [review]
patch v1

Thanks Paul - this sounds like manageable risk in that case.

Comment 12

[Paul Rouget [:paul]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/376035760ee0

Comment 13

[Paul Rouget [:paul]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3040211d5324