1150814 - Rulers graduations are not displayed

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

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

It seems that the CSS workaround used to have 1px stroke width for the graduations, is broken in some platform. As result, the graduations are not displayed at all.

Comment 1

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

Attachment: fix-rulers-stroke.patch

Comment 2

[Patrick Brosset <:pbro>]

Comment on attachment 8587855 [details] [diff] [review]
fix-rulers-stroke.patch

Review of attachment 8587855 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/server/actors/highlighter.js
@@ +2634,5 @@
> +
> +  this._zoom = 0;
> +
> +  this._cancelUpdate();
> +  this._update();

Why call both _cancelUpdate and _update here?
_cancelUpdate will have no effect and _update will do a bunch of stuff, including changing markup and starting the rAF loop, even though the highlighter is not shown yet.
You should only do all of this in show, and cancel the update in hide.

@@ +2810,5 @@
> +
> +  update: function() {
> +    let prefix = this.ID_CLASS_PREFIX;
> +    let { documentElement } = this.win.document;
> +    let zoom = LayoutHelpers.getCurrentZoom(documentElement);

I think LayoutHelpers.getCurrentZoom(this.win) should work just as well, no need to get the documentElement if I remember correctly.

Comment 3

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

(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #2)

> @@ +2810,5 @@
> > +
> > +  update: function() {
> > +    let prefix = this.ID_CLASS_PREFIX;
> > +    let { documentElement } = this.win.document;
> > +    let zoom = LayoutHelpers.getCurrentZoom(documentElement);
> 
> I think LayoutHelpers.getCurrentZoom(this.win) should work just as well, no
> need to get the documentElement if I remember correctly.

You remember correctly; however I used `documentElement` here 'cause I need it anyway later for the `setIgnoreLayoutChanges` call. If you prefer, I can just pass `this.win` here.

Comment 5

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

Attachment: Rulers graduations are not displayed

Comment 6

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

Comment on attachment 8639315 [details] [diff] [review]
Rulers graduations are not displayed

I improved a bit the previous patch; plus I added a check in gcli in case the window is closed / destroyed, to avoid exceptions.

Comment 7

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

Comment on attachment 8639315 [details] [diff] [review]
Rulers graduations are not displayed

Changing reviewer 'cause Patrick will be on PTO for a while.

Mike, can you have a look at that? If you don't feel comfortable to review this, just let me know! Thanks!

Comment 8

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

Attachment: Rulers graduations are not displayed

Comment 9

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

Comment on attachment 8644799 [details] [diff] [review]
Rulers graduations are not displayed

Just edited the commit's comment appending "r=mratcliffe" instead of patrick

Comment 10

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=34408a15465b

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/1102281dff11

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1102281dff11

Comment 13

[Saheda Reza [:Antora]]

Reproduce this bug on Firefox nightly Version 40.0a1

The bug is verified and fix on Latest Nightly

Build ID 	20150826030211
User Agent 	Mozilla/5.0 (Windows NT 6.3; rv:43.0) Gecko/20100101 Firefox/43.0

Tested OS -- Windows 8.1 32bit

Comment 14

[magicp]

Should the Rulers be changed direction in the RTL locale version? If yes, it should be changed.