XML prettyprinter no longer drops the tree styling when the DOM is modified

RESOLVED FIXED in mozilla25

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mozilla.mailt.hradek, Assigned: bzbarsky)

Tracking

22 Branch
mozilla25
x86
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:22.0) Gecko/20100101 Firefox/22.0 (Beta/Release)
Build ID: 20130618035212

Steps to reproduce:

For a long time I used a greasemonkey script to add some CSS to an XML file because it was a bit tedious to read the pure XML.

The script goes like this (stripped down a bit):

// ==UserScript==
// @name           checkloaded with style
// @namespace      checkloaded
// @include        */...my url here
// @grant          GM_setValue
// @grant          GM_getValue
// @grant          GM_deleteValue
// ==/UserScript==

if (GM_getValue(document.location.href, 0) == 1) {
     GM_deleteValue (document.location.href);
}
else {
    pi=document.createProcessingInstruction('xml-stylesheet', 'href="#" type="text/css"')
   
    document.documentElement.parentNode.insertBefore( pi, document.documentElement );
   
    var resultcode= document.getElementsByTagName('result-code')[0];
    resultcode.addEventListener("click", function() {
            GM_setValue(document.location.href, 1);
            history.go(0);
    }, true);

    resultcode.setAttribute("title", "Click to see raw XML");
    window.setTimeout(
        function() {
            var i=0;
            var css=document.styleSheets[0];
            css.rel='stylesheet';
            css.insertRule("response { \
                display: block;        \
                font-family: Arial;    \
                padding:2px;           \
                font-size: 8pt;        \
                color: white;             \
                background-color:#e60000; \
            }", i++);
            css.insertRule("response:after { \
                content: \"© by me \"; \
            }", i++);
            css.insertRule("result-code { \
                display: inline;           \
                padding:2px 2px 2px 20px; \
                color: white;             \
                font-size: 3em;           \
                cursor: pointer;          \
            }", i++);
            css.insertRule("loaded-list {            \
                display: table;                      \
                margin-left:auto; margin-right:auto; \
                border-collapse: collapse;           \
                color: black;             \
            }", i++);
            css.insertRule("property:nth-child(even) { \
                display: table-row;                   \
                background-color: #d0d0d0;            \
            }", i++);
            css.insertRule("property:nth-child(odd) { \
                display: table-row;                    \
                background-color: white;               \
            }", i++);
            css.insertRule("name {            \
                display: table-cell;          \
                font-weight: bold;            \
                padding: 3px;                 \
                min-width:20px;               \
                border-right: grey 1px solid; \
            }", i++);
            css.insertRule("mem-value { \
                display: table-cell;       \
                padding: 3px;              \
                min-width: 40px;           \
            }", i++);
    }, 1000);
}

And the XML looks like this:

<?xml version="1.0" encoding="UTF-8"?>
<response>
    <result-code>OK</result-code>
    <loaded-list>
        <property>
            <name>sp.invalid.request.message</name>
            <mem-value>The request is invalid.  Please contact the Service Portal administrator.</mem-value>
        </property>
        <property>
            <name>maintenance.thread.sleep.seconds</name>
            <mem-value>30</mem-value>
        </property>
        <property>
            <name>log_level</name>
            <mem-value>ERROR</mem-value>
        </property>
        <property>
            <name>cookie.secure</name>
            <mem-value>false</mem-value>
        </property>
        <property>
            <name>metrics.enable</name>
            <mem-value>true</mem-value>
        </property>
    </loaded-list>
</response>

The strange thing is: Partly it works as I can see the font change and shrink one second after loading the page.

The script is confirmed to still work with FF18: https://groups.google.com/forum/#!topic/greasemonkey-users/GmltyTCvdDU


Actual results:

I see the XML tree with just a changed font.


Expected results:

I should see a table containing all my XML entries
(Reporter)

Updated

5 years ago
Summary: XML is no longer rendered with CSS after upgrade to 21 → XML is no longer rendered with CSS after upgrade to 22
Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core
Does it work if you do href="data:text/css," instead of href="#"?  In my local test in an XML file (not using GreaseMonkey) that seems to work just fine.

Can you attach the actual stripped-down GM script you're using using https://bugzilla.mozilla.org/attachment.cgi?bugid=890193&action=enter just so we don't run into weird whitespace issues cutting and pasting and I can make sure I'm testing what you're testing?
(Reporter)

Comment 2

5 years ago
Created attachment 772571 [details]
Greasemonkey script to apply CSS to some XML

As requested in https://bugzilla.mozilla.org/show_bug.cgi?id=890193 by Boris Zbarsky (:bz)
(Reporter)

Comment 3

5 years ago
Hi Boris!
I'm not doing any "href" at all.

I receive the XML from a servlet and only apply the CSS by adding some elements to the XML DOM tree. I don't change the XML in any other way and I also do not have the possibility to change t.

I uploaded the stripped down GreaseMonkey code. Stripped down in this case means: Removed the (C) remark added in the final output and removed an image which also is usually added. So just 2 CSS rules are removed.
> I'm not doing any "href" at all.

Sure you are.  It's right here:

    pi=document.createProcessingInstruction('xml-stylesheet', 'href="#" type="text/css"')

Thank you for the script; I'll take a look.
OK, I can reproduce the behavior in Firefox 22: we keep showing the XML tree viewer instead of dropping the tree binding.  I had missed that part of comment 0; my test in comment 1 involved an XML file which did not get the tree viewer at all.

Looks like the behavior first changed in Firefox 21, in particular changed in this checkin range back in January 2013: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=98ea4d294369&tochange=a207f33adc1a

This looks like a likely regression from bug 833412.  Apparently we don't have sane tests for the XML prettyprinter dropping the binding?  :(
Blocks: 833412
Status: UNCONFIRMED → NEW
Component: CSS Parsing and Computation → XBL
Ever confirmed: true
Summary: XML is no longer rendered with CSS after upgrade to 22 → XML prettyprinter no longer drops the tree styling when the DOM is modified
So we land in nsBindingManager::ClearBinding, but binding->IsStyleBinding() is true so we return without doing anything.

In fact, it looks like there are no more callers of SetIsStyleBinding.  The only caller used to be in LoadBindings when aAugmentFlag was true, but that got nuked in bug 833412.
And I can't find a way to write an automated test for this, because the prettyprinter only triggers on toplevel windows....
Created attachment 772702 [details] [diff] [review]
Make the XML prettyprinter actually drop its binding when it unhooks.
Attachment #772702 - Flags: review?(bobbyholley+bmo)
Assignee: nobody → bzbarsky
Stephan, thank you for reporting this!
Flags: in-testsuite?
Whiteboard: [need review]
Comment on attachment 772702 [details] [diff] [review]
Make the XML prettyprinter actually drop its binding when it unhooks.

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

r=bholley
Attachment #772702 - Flags: review?(bobbyholley+bmo) → review+
And can you please add some comments explaining what IsStyleBinding means?
Created attachment 772735 [details] [diff] [review]
Alternate approach: just treat all bindings as style bindings
Attachment #772735 - Flags: review?(mrbkap)
Attachment #772702 - Attachment is obsolete: true
(Reporter)

Comment 14

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #4)
> > I'm not doing any "href" at all.
> 
> Sure you are.  It's right here:
> 
>     pi=document.createProcessingInstruction('xml-stylesheet', 'href="#"
> type="text/css"')

Oops :( I wrote this long and partly borrowed it. I should have looked more carefully.
For what it's worth, you may still want to change the script: it's loading the entire XML document a second time just to throw it away....  href="data:text/css," should work way better.
(Reporter)

Comment 16

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #15)
> For what it's worth, you may still want to change the script: it's loading
> the entire XML document a second time just to throw it away.... 
> href="data:text/css," should work way better.

I will try that. At the moment I'm doing XSLT transformations to ge the desired result.

But I don't understand your comment "it's loading the entire XML document a second time just to throw it away". Do you mean because of the "history.go(0);"?

This is just upon a click of the user in case he wants to see the XML tree. In that case the css is not applied.
Attachment #772735 - Flags: review?(mrbkap) → review+
> I will try that. 

It won't help this bug's issue, just the general performance of the script.

> Do you mean because of the "history.go(0);"?

No, because <?xml-stylesheet type="text/css" href="#"?> will load the entire XML document again from the network (or cache, depending), try to parse it as CSS, discover it's not CSS, and leave you with an empty stylesheet...
(Reporter)

Comment 18

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #17)
> > I will try that. 
> 
> It won't help this bug's issue, just the general performance of the script.
Tried it and it's really great. If the bug were not there It would be fantastic.

Thanks for telling me!
https://hg.mozilla.org/integration/mozilla-inbound/rev/86702cdd814c
Whiteboard: [need review]
Target Milestone: --- → mozilla25
Depends on: 892638
And https://hg.mozilla.org/integration/mozilla-inbound/rev/e46098561cb1 because this unhooking business asserts when it happens.  See bug 892638.
(In reply to neil@parkwaycc.co.uk from comment #4 in Bug 892638)
> This appears to have had the same cause as bug 943804, so I backed out the
> assertion annotations:
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/eef2361b1f9b
> 
> The test case no longer asserts either.

also hg commit comment Backout of changeset e46098561cb1 because bug 943804 fixed bug 892638 CLOSED TREE
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
_One_ of the changesets was backed out.  The one that doesn't actually fix this bug.  So it's still fixed just fine.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.