Rulers graduations are not displayed

RESOLVED FIXED in Firefox 42

Status

RESOLVED FIXED
3 years ago
2 months ago

People

(Reporter: zer0, Assigned: zer0)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 42
x86
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(firefox40 affected, firefox42 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
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.
(Assignee)

Updated

3 years ago
Assignee: nobody → zer0
(Assignee)

Comment 1

3 years ago
Created attachment 8587855 [details] [diff] [review]
fix-rulers-stroke.patch
Attachment #8587855 - Flags: review?(pbrosset)
(Assignee)

Updated

3 years ago
Blocks: 1150864
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.
Attachment #8587855 - Flags: review?(pbrosset)
(Assignee)

Comment 3

3 years ago
(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.

Updated

3 years ago
Duplicate of this bug: 1185238
(Assignee)

Comment 5

3 years ago
Created attachment 8639315 [details] [diff] [review]
Rulers graduations are not displayed
(Assignee)

Updated

3 years ago
Attachment #8587855 - Attachment is obsolete: true
(Assignee)

Comment 6

3 years ago
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.
Attachment #8639315 - Flags: review?(pbrosset)
(Assignee)

Updated

3 years ago
Blocks: 1152330
(Assignee)

Comment 7

3 years ago
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!
Attachment #8639315 - Flags: review?(pbrosset) → review?(mratcliffe)
Attachment #8639315 - Flags: review?(mratcliffe) → review+
(Assignee)

Comment 8

3 years ago
Created attachment 8644799 [details] [diff] [review]
Rulers graduations are not displayed
(Assignee)

Comment 9

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

Just edited the commit's comment appending "r=mratcliffe" instead of patrick
Attachment #8644799 - Flags: review+
(Assignee)

Updated

3 years ago
Attachment #8639315 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1102281dff11
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
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
QA Whiteboard: [bugday-20150826]

Comment 14

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

Updated

2 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.