[Webkit-unassigned] [Bug 13807] XML without style should render as syntax-highlighted source

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=13807


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #83838|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #33 from Pavel Feldman <pfeldman at chromium.org>  2011-02-25 14:20:06 PST ---
(From update of attachment 83838)
View in context: https://bugs.webkit.org/attachment.cgi?id=83838&action=review

Looks great. Please fix the JavaScript nits and we can land this.

> Source/WebCore/xml/XMLViewer.xsl:268
> +                function onload() {

should be on the next line. Can be fixed while landing.

> Source/WebCore/xml/XMLViewer.xsl:269
> +                  drawArrows();

Four spaces indent. Ditto.

> Source/WebCore/xml/XMLViewer.xsl:301
> +                function getElementsByClassName(classname, node, recursive)  {

Too many spaces before { (that should be on the next line)

> Source/WebCore/xml/XMLViewer.xsl:305
> +                    if (recursive) {

No need for { and } for one-liners.

> Source/WebCore/xml/XMLViewer.xsl:311
> +                        if(re.test(els[i].className)) {

Ditto

> Source/WebCore/xml/XMLViewer.xsl:319
> +                    getElementsByClassName('expanded', button.parentNode.parentNode.parentNode, false)[0].className = 'expanded';

what is button.parentNode.parentNode.parentNode? You should not walk DOM structure like that - it is better to store a link to the node you need into the button object itself.
Please use document.querySelectorAll() instead of this getElementsByClassName

> Source/WebCore/xml/XMLViewer.xsl:324
> +                    getElementsByClassName('collapsed', button.parentNode.parentNode.parentNode, false)[0].className = 'collapsed';

ditto

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.