Style inspector appears to ignore inline styles

RESOLVED FIXED in Firefox 10

Status

()

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

People

(Reporter: miker, Assigned: msucan)

Tracking

Trunk
Firefox 10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [styleinspector][minotaur])

Attachments

(1 attachment, 3 obsolete attachments)

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
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME
(Assignee)

Comment 2

6 years ago
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.
(Assignee)

Comment 5

6 years ago
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).
(Assignee)

Comment 6

6 years ago
(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.
(Assignee)

Comment 7

6 years ago
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!
Assignee: nobody → mihai.sucan
Status: REOPENED → ASSIGNED
Attachment #567471 - Flags: review?(dcamp)

Updated

6 years ago
Attachment #567471 - Flags: review?(dcamp) → review+
(Assignee)

Comment 8

6 years ago
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.
Attachment #567471 - Attachment is obsolete: true
Attachment #567994 - Flags: review?(dcamp)
(Assignee)

Updated

6 years ago
Blocks: 691736
No longer depends on: 692543
Version: unspecified → Trunk
(Assignee)

Comment 9

6 years ago
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.
Attachment #567994 - Attachment is obsolete: true
Attachment #567994 - Flags: review?(dcamp)
Attachment #569742 - Flags: review?(dcamp)
(Assignee)

Updated

6 years ago
No longer blocks: 691736

Comment 10

6 years ago
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+
(Assignee)

Comment 11

6 years ago
Created attachment 569751 [details] [diff] [review]
[in-fx-team] updated patch

Thanks for the r+! Updated the comment.
Attachment #569742 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Whiteboard: [styleinspector][minotaur] → [styleinspector][minotaur][land-in-fx-team]
(Assignee)

Comment 12

6 years ago
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
(Assignee)

Updated

6 years ago
Whiteboard: [styleinspector][minotaur][land-in-fx-team] → [styleinspector][minotaur][fixed-in-fx-team]

Comment 13

6 years ago
https://hg.mozilla.org/mozilla-central/rev/573567f90ed0
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Whiteboard: [styleinspector][minotaur][fixed-in-fx-team] → [styleinspector][minotaur]
Target Milestone: --- → Firefox 10
You need to log in before you can comment on or make changes to this bug.