692400 - Style inspector appears to ignore inline styles

Status: RESOLVED FIXED

(DevTools :: General, defect, P1)

Description

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

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.

Comment 1

[Florin Strugariu [:Bebe]]

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

Comment 2

[Mihai Sucan [:msucan]]

Mike: this bug is WFM on the current fx-team repo. Did you report this bug against your patch queue? Or ...?

Comment 3

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

Yes, it is somehow caused by get[un]matchedSelectors() ... bug 685902. The implementation from bug 692543 does not fix it.

Comment 4

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

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.

Comment 5

[Mihai Sucan [:msucan]]

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).

Comment 6

[Mihai Sucan [:msucan]]

(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.

Comment 7

[Mihai Sucan [:msucan]]

Attachment: proposed fix

Attaching the proposed changes.

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

Looking forward to your review. Thank you!

Comment 8

[Mihai Sucan [:msucan]]

Attachment: rebase on top of fx-team

Asking for a quick review since this is different code, not a true rebase.

The test is included in bug 691736.

Comment 9

[Mihai Sucan [:msucan]]

Attachment: rebase

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.

Comment 10

[Dave Camp (:dcamp)]

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.

Comment 11

[Mihai Sucan [:msucan]]

Attachment: [in-fx-team] updated patch

Thanks for the r+! Updated the comment.

Comment 12

[Mihai Sucan [:msucan]]

Comment on attachment 569751 [details] [diff] [review]
[in-fx-team] updated patch

Landed:
https://hg.mozilla.org/integration/fx-team/rev/573567f90ed0

Comment 13

[:Margaret Leibovic]

https://hg.mozilla.org/mozilla-central/rev/573567f90ed0