Last Comment Bug 692400 - Style inspector appears to ignore inline styles
: Style inspector appears to ignore inline styles
Status: RESOLVED FIXED
[styleinspector][minotaur]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: Firefox 10
Assigned To: Mihai Sucan [:msucan]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-06 02:30 PDT by Michael Ratcliffe [:miker] [:mratcliffe]
Modified: 2011-10-27 11:45 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
proposed fix (7.79 KB, patch)
2011-10-17 09:13 PDT, Mihai Sucan [:msucan]
dcamp: review+
Details | Diff | Review
rebase on top of fx-team (2.72 KB, patch)
2011-10-19 03:20 PDT, Mihai Sucan [:msucan]
no flags Details | Diff | Review
rebase (7.77 KB, patch)
2011-10-26 11:27 PDT, Mihai Sucan [:msucan]
dcamp: review+
Details | Diff | Review
[in-fx-team] updated patch (8.08 KB, patch)
2011-10-26 11:56 PDT, Mihai Sucan [:msucan]
no flags Details | Diff | Review

Description Michael Ratcliffe [:miker] [:mratcliffe] 2011-10-06 02:30:29 PDT
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] 2011-10-12 04:56:06 PDT
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] 2011-10-12 07:16:17 PDT
Mike: this bug is WFM on the current fx-team repo. Did you report this bug against your patch queue? Or ...?
Comment 3 Michael Ratcliffe [:miker] [:mratcliffe] 2011-10-14 04:48:36 PDT
Yes, it is somehow caused by get[un]matchedSelectors() ... bug 685902. The implementation from bug 692543 does not fix it.
Comment 4 Michael Ratcliffe [:miker] [:mratcliffe] 2011-10-14 10:16:29 PDT
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] 2011-10-14 12:31:11 PDT
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] 2011-10-14 12:36:32 PDT
(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] 2011-10-17 09:13:45 PDT
Created attachment 567471 [details] [diff] [review]
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] 2011-10-19 03:20:34 PDT
Created attachment 567994 [details] [diff] [review]
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] 2011-10-26 11:27:10 PDT
Created attachment 569742 [details] [diff] [review]
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) 2011-10-26 11:33:56 PDT
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] 2011-10-26 11:56:49 PDT
Created attachment 569751 [details] [diff] [review]
[in-fx-team] updated patch

Thanks for the r+! Updated the comment.
Comment 12 Mihai Sucan [:msucan] 2011-10-27 03:50:17 PDT
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 2011-10-27 11:45:49 PDT
https://hg.mozilla.org/mozilla-central/rev/573567f90ed0

Note You need to log in before you can comment on or make changes to this bug.