566096 - Inspect menu item's check mark gets out of sync when switching tabs

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[(no longer active)]

If you turns Inspect on on a tab, and switch to the other tab, the menu item remains checked.  If you click it again, Inspect is activated on the new tab and the menu item remains checked.  Clicking it again unchecks it.

Comment 1

[Rob Campbell [:rc] (:robcee)]

whups. Must have missed that case. Thanks.

Comment 2

[Rob Campbell [:rc] (:robcee)]

Attachment: inspect command checkmark

Comment 3

[Rob Campbell [:rc] (:robcee)]

This patch moves the update to the individual closeInspectorUI and openInspectorUI methods from toggleInspectorUI where it was being missed on TabSelect.

this could use a simple test to go along with it.

Comment 4

[(no longer active)]

Did you mean to add helpwanted on this bug?

Comment 5

[Rob Campbell [:rc] (:robcee)]

I would take help if it was offered!

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 445949 [details] [diff] [review]
inspect command checkmark

Probably wouldn't hurt to add a smart getter for this:

XPCOMUtils.defineLazyGetter(InspectorUI, "inspectCmd", function () {
  return document.getElementById("Tools:Inspect");
});

and then just use this.inspectCmd everywhere.

(If you don't do that, put the toolsInspectCmd declarations just before their use, rather than at the beginning of the function.)

Comment 7

[Rob Campbell [:rc] (:robcee)]

I like this. Should I be doing this for the other calls to getElementById() in the InspectorUI constructor?

Comment 8

[Rob Campbell [:rc] (:robcee)]

s/constructor/opener/ in the last comment.

Comment 9

[Rob Campbell [:rc] (:robcee)]

Attachment: inspect command checkmark

updated patch. Ready for checkin.

Comment 10

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 446222 [details] [diff] [review]
inspect command checkmark

(r+ from gavin in previous patch, just carried it forward)

Comment 11

[Rob Campbell [:rc] (:robcee)]

Attachment: inspect command checkmark

review carried forward. ready to checkin.

Comment 12

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 446515 [details] [diff] [review]
inspect command checkmark

checked-in

changeset:   42501:6033fff0ffd0