1173298 - Add rule button should be disabled for anonymous elements or non-element nodes

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Comment 1

[Tim Nguyen :ntim]

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

Comment 2

[Tim Nguyen :ntim]

Attachment: Patch

Try push in previous comment

Comment 3

[Tim Nguyen :ntim]

Attachment: Patch v1.1

Small change in test.

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8617860 [details] [diff] [review]
Patch v1.1

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

Looks good, would just like to see the test expanded to cover anonymous elements

::: browser/devtools/styleinspector/test/browser_ruleview_add-rule_04.js
@@ +3,5 @@
> + http://creativecommons.org/publicdomain/zero/1.0/ */
> +
> +"use strict";
> +
> +// Tests the behaviour of adding a new rule to the rule view and the

Please update this comment to reflect what the test is doing

@@ +31,5 @@
> +  ok(view.addRuleButton.disabled, "Add rule button should be disabled");
> +
> +  info("Selecting a real element");
> +  yield selectNode(node, inspector);
> +  ok(!view.addRuleButton.disabled, "Add rule button should be enabled");

You can also test an anonymous child by doing something like:

  <style type="text/css">
    #pseudo::before {
      content: "before";
    }
  </style>
  <div id="pseudo"><span></span></div>

  let pseudo = yield getNodeFront("#pseudo", inspector);
  let children = yield inspector.walker.children(pseudo);
  let before = children.nodes[0];

  // select node, check disabled state on button

Comment 5

[Tim Nguyen :ntim]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0ab735d2c90f

Comment 6

[Tim Nguyen :ntim]

Attachment: Patch v2

Haven't ran the test locally yet (Firefox is still building)

Comment 7

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] from comment #6)
> Haven't ran the test locally yet (Firefox is still building)
Passes fine locally.

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8620376 [details] [diff] [review]
Patch v2

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

We should have some additional styling for disabled .devtools-button elements that are just using an icon.  Can't tell by looking at it that it's disabled.   Maybe some opacity on devtools-button[disabled]::before... what do you think?

Comment 9

[Tim Nguyen :ntim]

Attachment: Patch v2.1

Added opacity: 0.5; for disabled buttons.

Comment 10

[Tim Nguyen :ntim]

Green try in comment 5

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/7517bf3c8b5c

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7517bf3c8b5c