1260043 - "Toggle rulers for the page" is broken if toolbox is detached

Status: RESOLVED FIXED

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

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 48, 32bit, ID 20160326030430
STR:
1. Open http://example.org/ in a new tab
2. Open devtools, dock it to the bottom side of the window 
3. Open https://ya.ru/ in a new tab   (optional: open devtools in that tab)
4. Switch to the tab from Step 1, detach toolbox in separate window
5. Switch to the tab from Step 3
6. Switch to devtools window from Step 4
7. Click "Toggle rulers for the page" button
8. Switch to the tab from Step 1

AR:
 No rulers

ER:  Either X or Y or Z
 X) After Step 8 rulers should be toggled for the tab from Step 1
 Y) After Step 7 browser window should be placed below toolbox window and the tab from Step 1
    should be selected
 Z) After Step 7 an alert/popup panel/something else should say that tab from Step 1 is not visible

See also explanation of (Y) in bug 1251863 comment 6

Comment 1

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

I filed bug 1267219, fixing that should fixes both this bug and bug 1260044.

Comment 2

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

Attachment: MozReview Request: Bug 1260043 - removed `doc.hidden` check from markup/utils; r=pbro

Review commit: https://reviewboard.mozilla.org/r/56916/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56916/

Comment 3

[Patrick Brosset <:pbro>]

Comment on attachment 8758703 [details]
MozReview Request: Bug 1260043 - removed `doc.hidden` check from markup/utils; r=pbro

I think Alex would be interested in reviewing this since he was the one who worked on this part of the code if I remember correctly.
Alex, you should read Bug 1267219 comment 11 before I think.

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8758703 [details]
MozReview Request: Bug 1260043 - removed `doc.hidden` check from markup/utils; r=pbro

https://reviewboard.mozilla.org/r/56916/#review53906

I can't find any safe check that isn't document.hidden.
Nor can I find of any real case where similar thing would happen on desktop.
But that's great to keep this comment in case it starts to throw again.

The only case where it may fail is for documents from the bfcache, but I don't see why we would have an highlighter against such document as TabActor throws events when we switch into/off the bfcache...

So if try is green, let's just remove this.

Comment 5

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

The orange doesn't seems related to this patch:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d65faa064c5&selectedJob=21935870

So I'm going to land it. Thanks for the review Alex!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/99dacc787a59
removed `doc.hidden` check from markup/utils; r=ochameau

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/99dacc787a59