Closed Bug 692400 Opened 8 years ago Closed 8 years ago

Style inspector appears to ignore inline styles

Categories

(DevTools :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 10

People

(Reporter: miker, Assigned: msucan)

Details

(Whiteboard: [styleinspector][minotaur])

Attachments

(1 file, 3 obsolete files)

Steps to reproduce:
1. Go to http://mihaisucan.github.com/mozilla-work/test.html
2. Open the web console
3. inspectstyle($('bigarea'))

What happens?
The bigarea code looks like this:
<div id="bigarea" style="border: 1px red solid">big area</div>

No styles are found when the "Only user styles" checkbox is checked. When the checkbox is unchecked the element's inline styles are not listed under any of the border properties.

What should happen?
Inline styles should be listed.
works for me on:
Mozilla/5.0 (Windows NT 6.1; rv:9.0a2) Gecko/20111011 Firefox/9.0a2
and
Mozilla/5.0 (Windows NT 6.1; rv:10.0a1) Gecko/20111011 Firefox/10.0a1
the inline stiles are visible on both versions 

http://screencast.com/t/ANbMwm2GW4E
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Mike: this bug is WFM on the current fx-team repo. Did you report this bug against your patch queue? Or ...?
Yes, it is somehow caused by get[un]matchedSelectors() ... bug 685902. The implementation from bug 692543 does not fix it.
Status: RESOLVED → REOPENED
Depends on: 685902
Resolution: WORKSFORME → ---
Depends on: 692543
No longer depends on: 685902
The root cause is that domUtils.getCSSStyleRules() ignores inline styles. It also seems that there is no way to convert inline styles into css rules. It seems like we will need to do the following:
1. In CL__buildMatchedRules we need to also loop through this.viewedElement.style and add the properties to a new collection ... I suppose we also need to remember these rules in unmatchedRules.
2. In CL_hasMatchedSelectors we need to also take these inline styles into account.
3. When getting unmatched rules we also need to take inline rules into account.

In my opinion getCSSStyleRules should also include information about inline styles.
Mike: thank you for the bug report! Just checked the code. I will fix the issue in bug 692543, it's only a few lines of code (or here, depends how dcamp wants).
(In reply to Michael Ratcliffe from comment #4)
> The root cause is that domUtils.getCSSStyleRules() ignores inline styles. It
> also seems that there is no way to convert inline styles into css rules. It
> seems like we will need to do the following:
> 1. In CL__buildMatchedRules we need to also loop through
> this.viewedElement.style and add the properties to a new collection ... I
> suppose we also need to remember these rules in unmatchedRules.
> 2. In CL_hasMatchedSelectors we need to also take these inline styles into
> account.
> 3. When getting unmatched rules we also need to take inline rules into
> account.

The element.style properties are always matched stuff, so this doesn't impact unmatched rules.

> In my opinion getCSSStyleRules should also include information about inline
> styles.

I wouldn't say that's a bug. element.style is CSSStyleDeclaration, not a CSSStyleRule.
Attached patch proposed fix (obsolete) — Splinter Review
Attaching the proposed changes.

This code makes the element.style properties visible for hasMatchedSelectors().

Looking forward to your review. Thank you!
Assignee: nobody → mihai.sucan
Status: REOPENED → ASSIGNED
Attachment #567471 - Flags: review?(dcamp)
Attachment #567471 - Flags: review?(dcamp) → review+
Attached patch rebase on top of fx-team (obsolete) — Splinter Review
Asking for a quick review since this is different code, not a true rebase.

The test is included in bug 691736.
Attachment #567471 - Attachment is obsolete: true
Attachment #567994 - Flags: review?(dcamp)
Blocks: 691736
No longer depends on: 692543
Version: unspecified → Trunk
Attached patch rebase (obsolete) — Splinter Review
Rebased patch and folded the test from bug 691736.

Included a minor fix which I noticed during testing: the internal selector text "@element.style" was displayed when I used inspectstyle() from the Web Console.
Attachment #567994 - Attachment is obsolete: true
Attachment #567994 - Flags: review?(dcamp)
Attachment #569742 - Flags: review?(dcamp)
No longer blocks: 691736
Comment on attachment 569742 [details] [diff] [review]
rebase

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

::: browser/devtools/styleinspector/CssLogic.jsm
@@ +612,2 @@
>     *
>     * @param {function} [aCallback] Simple callback method

Might make it explicit that the callback returns a boolean indicating matched/unmatched.
Attachment #569742 - Flags: review?(dcamp) → review+
Thanks for the r+! Updated the comment.
Attachment #569742 - Attachment is obsolete: true
Whiteboard: [styleinspector][minotaur] → [styleinspector][minotaur][land-in-fx-team]
Comment on attachment 569751 [details] [diff] [review]
[in-fx-team] updated patch

Landed:
https://hg.mozilla.org/integration/fx-team/rev/573567f90ed0
Attachment #569751 - Attachment description: updated patch → [in-fx-team] updated patch
Whiteboard: [styleinspector][minotaur][land-in-fx-team] → [styleinspector][minotaur][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/573567f90ed0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Whiteboard: [styleinspector][minotaur][fixed-in-fx-team] → [styleinspector][minotaur]
Target Milestone: --- → Firefox 10
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.