Closed
Bug 890193
Opened 11 years ago
Closed 11 years ago
XML prettyprinter no longer drops the tree styling when the DOM is modified
Categories
(Core :: XBL, defect)
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: mozilla.mailt.hradek, Assigned: bzbarsky)
References
Details
Attachments
(3 files, 1 obsolete file)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
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•11 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
Updated•11 years ago
|
Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core
Assignee | ||
Comment 1•11 years ago
|
||
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•11 years ago
|
||
As requested in https://bugzilla.mozilla.org/show_bug.cgi?id=890193 by Boris Zbarsky (:bz)
Reporter | ||
Comment 3•11 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.
Assignee | ||
Comment 4•11 years ago
|
||
> 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.
Assignee | ||
Comment 5•11 years ago
|
||
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
Assignee | ||
Comment 6•11 years ago
|
||
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.
Assignee | ||
Comment 7•11 years ago
|
||
And I can't find a way to write an automated test for this, because the prettyprinter only triggers on toplevel windows....
Assignee | ||
Comment 8•11 years ago
|
||
Attachment #772702 -
Flags: review?(bobbyholley+bmo)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → bzbarsky
Assignee | ||
Comment 9•11 years ago
|
||
Stephan, thank you for reporting this!
Flags: in-testsuite?
Whiteboard: [need review]
Comment 10•11 years ago
|
||
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+
Comment 11•11 years ago
|
||
And can you please add some comments explaining what IsStyleBinding means?
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #772735 -
Flags: review?(mrbkap)
Assignee | ||
Updated•11 years ago
|
Attachment #772702 -
Attachment is obsolete: true
Assignee | ||
Comment 13•11 years ago
|
||
Reporter | ||
Comment 14•11 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.
Assignee | ||
Comment 15•11 years ago
|
||
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•11 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.
Updated•11 years ago
|
Attachment #772735 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 17•11 years ago
|
||
> 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•11 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!
Assignee | ||
Comment 19•11 years ago
|
||
Whiteboard: [need review]
Target Milestone: --- → mozilla25
Assignee | ||
Comment 20•11 years ago
|
||
And https://hg.mozilla.org/integration/mozilla-inbound/rev/e46098561cb1 because this unhooking business asserts when it happens. See bug 892638.
Comment 21•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/86702cdd814c
https://hg.mozilla.org/mozilla-central/rev/e46098561cb1
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 22•11 years ago
|
||
(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 → ---
Comment 23•11 years ago
|
||
and so also backed out from m-c in https://hg.mozilla.org/mozilla-central/rev/eef2361b1f9b
Assignee | ||
Comment 24•11 years ago
|
||
_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
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•