668435 - Inspector's hilight box should round corners to match what it's hilighting

Status: RESOLVED WONTFIX

(DevTools :: Inspector, defect)

Description

[Justin Dolske [:Dolske]]

The Inspector would ever ever-so-slightly more awesome than it already is if the hilight box sometimes had rounded corners.

Specifically, if it checked getComputedStyle() on the thing it was hilighting, and rounded it's own corners to match. That would make it feel even more like it was hilighting the actual element in the page, as opposed to a floating square awesomebox.

Comment 1

[Paul Rouget [:paul]]

I like that.

Comment 2

[Sergiu Dumitriu]

Attachment: Patch addressing the issue

Patch addressing the issue. It doesn't look perfect because of bug 382721. It uses the non-standard MozOutlineRadius property to also round the dashed outline.

Test for this issue included, but I noticed that although the test includes a 500ms delay to "make sure the highlighter is not mutting the resize event", it still fails because the outline is not refreshed on time. At least on my machine it takes about 4 seconds until a refresh actually happens.

Comment 3

[Sergiu Dumitriu]

Attachment: Patch for inclusion

Comment 4

[Paul Rouget [:paul]]

Thanks for the patch. I'll try to review that next week after the merge.

Comment 5

[Sergiu Dumitriu]

Another thing that I'm not happy about is that I've added a scaleValue method [1] which seems generic enough to be included in a common module/library. Is there already such a method that I can use instead?

[1] https://bugzilla.mozilla.org/attachment.cgi?id=697916&action=diff#a/browser/devtools/inspector/Highlighter.jsm_sec5

Comment 6

[Paul Rouget [:paul]]

Comment on attachment 697916 [details] [diff] [review]
Patch for inclusion

The radius should be computed in getDirtyRect.

You're not sure that the whole rectangle is visible. What about situations like that: http://jsbin.com/obosan ?

Comment 7

[Paul Rouget [:paul]]

Rethinking it, that sounds a bit hard to fix. Removing [good-first-bug].

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Because we are planning on always using the box model I don't believe that this will apply (the box model is always square unless transformed).

Comment 9

[Joe Walker [:jwalker]]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #8)
> Because we are planning on always using the box model I don't believe that
> this will apply (the box model is always square unless transformed).

Is the area just 'outside' of a rounded corner (but still inside of the border-box) part of the content/padding or the margin? It's not colored to match the content/padding so there's a good argument to say that it's part of the margin, and should be highlighted as such. i.e. the highlight box should have rounded corners to match border-radius.

Comment 10

[Gabriel Luong [:gl] (ΦωΦ)]

Marking this as WONTFIX, I believe the highlighting currently available in FFN45 is sufficient without rounding the corners