The default bug view has changed. See this FAQ.

Remove unmatched rules from style inspector

RESOLVED FIXED in Firefox 10

Status

()

Firefox
Developer Tools
P1
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: miker, Assigned: miker)

Tracking

unspecified
Firefox 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [style-inspector][fixed-in-fx-team])

Attachments

(2 attachments, 2 obsolete attachments)

The calculations involved with unmatched rules are very costly so we need to drop them.
Depends on: 692543
Created attachment 571316 [details] [diff] [review]
Unmatched selector extraction

Now even fast on Github! I tried removing the timeouts but then the interface became far less responsive. Works fine with breadcrumbs etc.
Attachment #571316 - Flags: review?(mihai.sucan)
Comment on attachment 571316 [details] [diff] [review]
Unmatched selector extraction

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

Patch looks good, r+. Thank you!


Non-review comment: I would have expected a minimal patch that simply removes the calls to code dealing with unmatched selectors - basically just remove them from the UI. This would have made the review quicker and it would have made easier to later revive unmatched selectors. Since the work is already done in this patch, no point in going back.

Performance is instant for me on ubuntu.com and github.com. Yay!
Attachment #571316 - Flags: review?(mihai.sucan) → review+
Whiteboard: [styleinspector] → [styleinspector][land-in-fx-team]
Created attachment 572188 [details] [diff] [review]
Rebased
Attachment #571316 - Attachment is obsolete: true
Blocks: 689759
https://hg.mozilla.org/integration/fx-team/rev/2967e93cdb44
Whiteboard: [styleinspector][land-in-fx-team] → [styleinspector][fixed-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/f0e736dbc7bc

https://hg.mozilla.org/integration/fx-team/rev/1b6acffa7a94
Status: NEW → ASSIGNED
Whiteboard: [styleinspector][fixed-in-fx-team] → [style-inspector][backed-out]

Comment 6

6 years ago
Backed out due to rebase errors slipping in to the css files.
Created attachment 572232 [details] [diff] [review]
minimal patch to disable unmatched selectors

As agreed with dcamp, I have made a minimal patch to disable the unmatched selectors.

This patch now depends on bug 700036.
Attachment #572232 - Flags: review?(rcampbell)

Updated

6 years ago
Depends on: 700036

Updated

6 years ago
Blocks: 700061
and further backing out:

https://hg.mozilla.org/integration/fx-team/rev/b5d68f1763ed
and also:

https://hg.mozilla.org/integration/fx-team/rev/90b03b0aa9ee

Comment 10

6 years ago
Comment on attachment 572232 [details] [diff] [review]
minimal patch to disable unmatched selectors

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

We should either:

a) Make sure Every Commented-out-section has a comment with the followup
b) Replace comments with if (kUnmatchedEnabled) { ... }
c) Just remove the minimal UI glue code (but not the bulk of the unmatchedSelector support in CssLogic) and make sure the followup bug includes this patch in case someone wants to reference when they put stuff back in.

I slightly prefer c), commented out code tends to bit-rot.  Anyone resurrecting this support can put back in the minimal glue code manually.  But only slightly.

::: browser/devtools/styleinspector/CssHtmlTree.jsm
@@ +138,5 @@
>  CssHtmlTree.prototype = {
>    // Cache the list of properties that have matched and unmatched properties.
>    _matchedProperties: null,
> +
> +  // TODO: bug 700061 - re-enalbe unmatched selectors or entirely remove them

This typo is propagated through the patch
Created attachment 572278 [details] [diff] [review]
minimal patch 2

Updated minimal patch. I have removed the code instead of commenting out, as requested. I did not remove code that doesn't need to be removed for the purpose of disabling unmatched selectors. I am trying to keep the patch *minimal*, with minimal rebase churn. If we grow the patch we'll have more rebase fun.

(asking for review from any of you dcamp/robcee - who gets to this first is fine. thank you!)
Attachment #572232 - Attachment is obsolete: true
Attachment #572232 - Flags: review?(rcampbell)
Attachment #572278 - Flags: review?(rcampbell)
Attachment #572278 - Flags: review?(dcamp)

Updated

6 years ago
Priority: -- → P1
Comment on attachment 572278 [details] [diff] [review]
minimal patch 2

looks ok, still not keen on the comments. Should maybe reference future remove unmatched selectors bug in preceding comment.

R+ with that.
Attachment #572278 - Flags: review?(rcampbell) → review+

Comment 13

6 years ago
https://hg.mozilla.org/integration/fx-team/rev/74b8d49b12af
Whiteboard: [style-inspector][backed-out] → [style-inspector][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/74b8d49b12af
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10

Updated

5 years ago
Attachment #572278 - Flags: review?(dcamp)
You need to log in before you can comment on or make changes to this bug.