This assertion helper is currently weak. Today it just checks that the middle point of the content box model is a point of the expected DOM node. This is weak because we may be highlighting a parent or a child node and the assertion would still be true. Also, it can fail unexpectedly if the middle point happens to be on a child node of the asserted node. Ideally we would check that the boxmodel being displayed relates to the asserted node. This assertion happens to be failing on luciddream as the layout is a bit different.

Assert that each cardinal point of the node is within the box model and fix the tests that has broken/weak assertions. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e140464464ca

Comment on attachment 8665327 [details] [diff] [review] patch v1 I had to pull complex algorithm to do "point is in polygon" checks. I'm open to simplier checks if you know one.

Comment on attachment 8665327 [details] [diff] [review] patch v1 Review of attachment 8665327 [details] [diff] [review]: ----------------------------------------------------------------- Sorry Alex, I have just had too many things to do and too many patches to review in the past weeks, and still do, so I prefer forwarding this over to Matteo (who now knows as much as I do about highlighters) to avoid making you wait even longer.

Comment on attachment 8665327 [details] [diff] [review] patch v1 Review of attachment 8665327 [details] [diff] [review]: ----------------------------------------------------------------- Look good to me! Nothing really to say, I just highlighted some minor stuff, that are more about readability, so it's up to you. ::: devtools/client/shared/test/test-actor.js @@ +664,5 @@ > + return true; > + } > + if (newPoints[i][1] <= point[1]) { > + if (newPoints[i + 1][1] > point[1]) { > + if (r > 0) { It seems to me we could use `&&` here, instead of nested `if` @@ +670,5 @@ > + } > + } > + } else { > + if (newPoints[i + 1][1] <= point[1]) { > + if (r < 0) { Same here. @@ +677,5 @@ > + } > + } > + } > + if (wn === 0) { > + dump(JSON.stringify(point)+" is outside of "+JSON.stringify(polygon)+"\n"); it should be available a `dumpn` function in `test-actor`, that automatically add `\n` at the end. @@ +687,5 @@ > + let {visible, border} = yield this._getBoxModelStatus(); > + let points = border.points; > + if (visible) { > + // Check that the node is within the box model > + let rect = yield this.getNodeRect(selector); maybe you could directly get `{ left, top, width, height }` instead of `rect`, so that below we'll have `[left, top], points` as we have `[right, bottom], points`.

Addressed all comments.

https://hg.mozilla.org/integration/fx-team/rev/d6fc1e8c81f22e76ffda8ab2ffc1728b92d46bdd Bug 1207977 - Ensure that node are correctly highlighted. r=zer0