Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > comp.lang.javascript > #18739

Re: Clean up HTML content

From Cezary Tomczyk <cezary.tomczyk@gmail.com>
Newsgroups comp.lang.javascript
Subject Re: Clean up HTML content
Date 2013-03-06 22:02 +0100
Organization Aioe.org NNTP Server
Message-ID <kh8ata$am8$1@speranza.aioe.org> (permalink)
References <kh4813$lkv$1@speranza.aioe.org> <2342178.BYbISWzQ9U@PointedEars.de> <kh5grn$bm0$1@speranza.aioe.org> <1699591.rPLLIT5s0r@PointedEars.de>

Show all headers | View raw


W dniu 2013-03-06 00:03, Thomas 'PointedEars' Lahn pisze:
> Cezary Tomczyk wrote:
>
>> W dniu 2013-03-05 17:55, Thomas 'PointedEars' Lahn pisze:
[...]
>> I know that this is Usenet, not the Web. I thought that code is too big
>> and instead of posting code here it would be better to prepare a working
>> example.
>
> You have posted 75 lines now.  Of those, unless I miscounted or
> miscalculated, 48 of them are source code.  I do not think that is “too big”
> for source code on this newsgroup.
>
> Consider that there have been discussions here with very valuable postings
> of 200 lines and more.  Although few if any of those postings contained
> source code in all or even the majority of lines, what is to discuss *here*
> if we adopt a rule that, say, posting code was inappropriate when it
> exceeded 40 lines?
>
> There is an individual limit as to the acceptable length of a posting, but I
> doubt it starts below 100 lines for anyone.  The same is true for source
> code in postings.  This posting is likely to have more than 100 lines,
> including source code.
>
> What is usually frowned upon is posting uncommented source code that is not
> self-explanatory, and maybe even badly formatted, because the poster did not
> take care to understand it in the first place.  Your code is neither.

I'll try to consider it while will be posting code next time. As for 
formatting, code was formatted, but for some reason do not know why here 
is unformatted.

[...]

>> Here's the code:
>>
>> var is_block_style = function (el) {
>>       var el_style;
>>       if (window.getComputedStyle) {
>
> You should feature-test this method in a way that does not primarily use
> implicit type conversion.  There is one DOM implementation, MSHTML, that is
> notorious for throwing strange exceptions on properties of type "unknown"
> otherwise.  See also, e.g., jsx.object.isHostMethod().

Yes, I know that IE return sometimes "object", sometimes "function". 
However, I had never any problems with implicit type conversion like above.

One request: if you refer to some methods of your code, eg. e.g., 
jsx.object.isHostMethod, then please also add link to source code. I 
guess it's coming from JSX project 
http://pointedears.de/websvn/listing.php?repname=JSX.

And you thought about that, right?

jsx.object.isMethodType = function (s) {
   return /^\s*(function|object|unknown)\s*$/i.test(s);
};

>>           el_style = window.getComputedStyle(el,
>> null).getPropertyValue('display');
>>       } else {
>>           el_style = el.currentStyle.display;
>>       }
>>       return el_style;
>> };
>
> You should generalize this code, see also jsx.dom.getComputedStyle().  In

I do not see here 
http://pointedears.de/websvn/filedetails.php?repname=JSX&path=%2Ftrunk%2Fdom.js 
method getComputedStyle.

> this case, because the base object always is the same, you can even safely
> perform the branching for the window.getComputedStyle() DOM method – which
> should be document.defaultView.getComputedStyle() to be compatible – only
> once.  You cannot assume, however, that if the getComputedStyle() method is
> absent, the “currentStyle“ property must be present; that is an inductive
> fallacy (hasty generalization).

Hm, both sources:

* https://developer.mozilla.org/en-US/docs/DOM/window.getComputedStyle
* http://msdn.microsoft.com/en-us/library/ie/ff975168%28v=vs.85%29.aspx

says about window.getComputedStyle. That's why I used this method.

However, when I look at David Mark method getKomputedStyle seems it's 
quite complicated :-(

> A method whose name starts with “is” should not return a string value, but a
> boolean value.  In this case, it should return the result of the comparison
> against any of those property values which should be considered indicative
> of a “block element” (see CSS 2.1, also CSS3 Box Model for details).

Makes sense. Thanks.

>> var imgClassName = 'testimg';
>>
>> var tidyHTML = function (node) {
>>       var elms = node.childNodes,
>>           i = elms.length,
>>           text = '',
>>           elm;
>>
>>       if (!node.hasChildNodes()
>
> I have never used that method before.  Unless you are sure about its
> backwards-compatibility, you should replace the call with

I use https://developer.mozilla.org/en-US/docs/DOM/Node.hasChildNodes 
from long time. Sesms it's implemented long time ago. See: 
http://www.quirksmode.org/dom/w3c_core.html

>    if (!node.childNodes.length > 0
>
> If you do not insist on an explicit condition, use
>
>    if (!node.childNodes.length
>
> or
>
>    if (!node.firstChild
>
> whereas the latter is probably the more efficient variant of the two.
> I think it is safe to assume that those values can be type-converted if
> necessary, because otherwise a lot of simple client-side scripts would fail.
> But if necessary and appropriate for your use-case, use
>
>    if (null != node.firstChild
>
> or
>
>    if (node.firstChild !== null
>
> In theory,
>
>    if (null !== node.firstChild
>
> should have the best runtime efficiency, returning in step 3 of the “Strict
> Equality Comparison Algorithm” (ES 5.1, 11.9.6) if node.firstChild is null,
> and returning in step 1 if it is not, already.  (I have not applied this
> knowledge yet.  Profiling will show if I should.)

This one:
if (!node.firstChild
is nice. However, as I wrote, I have never had problems with 
hasChildNodes method.

>>       && (node.nodeName.toLowerCase() === 'img'
>> && !(new RegExp('(^|\\s)' + imgClassName +
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> '(\\s|$)')).test(node.className))) {
>    ^^^^^^^^^^^
> You should move or even generalize this code in order to leverage caching.

Move new RegExp outside of method to do not creating new objects RegExp 
everytime?

> You can do that because the RegExp does not have the “global” flag set
> (i.e.,  its boolean ”global” property has the value “false”).

Right. I should set "global" as "true".

>>           node.parentNode.removeChild(node);
>>           return text;
>>       }
>>
>>       while (i--) {
>>           elm = elms[i];
>
> So AIUI you iterate backwards through the child nodes …

Yes.

>>           if (elm.nodeType === 1) {
>
> A strict comparison does not appear to be necessary or prudent here.  You
> cannot know that the types match since you are dealing with a host object.
> What you can know (from the ECMAScript Specification) is that *if* the types
> match, it does not matter for the result or efficiency if you use a strict
> comparison or not.  And you gain a little bit more backwards-compatibility
> that way.

Yes, I remember your explanation from the past. But it's about JSLint 
which I do not remember now why JSLint require "===" all the time.

>>               text += tidyHTML(elm);
>
> … but why do you *append* to the “text” variable as if you had iterated
> forwards, thereby reversing the order of the content?

Because sometimes I removing node.

> Equivalent code:
>
>    var a = ["foo", "bar", "baz"];
>    var i = a.length;
>    var a2 = [];
>    while (i--)
>    {
>      if (a[i] != "bar")
>      {
>        a2.push(a[i]);
>      }
>    }
>
>    /* ["baz", "foo"] */
>    console.log(a2);
>
> You are looking for something more like
>
>    for (var i = 0, len = elms.length; i < len; ++i)
>    {
>      text += tidyHTML(elms[i]);
>    }

Not really. In short: I want to remove all html nodes, except img with 
specified class name + I need to care about formatting, eg. if text is 
in block element then I apply <br/> after plain text.

>>           } else if (elm.nodeType === 3) {
>>               text += elm.data;
>>           } else if (elm.nodeType !== 3 && elm.nodeType !== 1) {
>>               elm.parentNode.removeChild(elm);
>>           }
>
> ISTM that at least one of those should happen before the loop, and before
> the function returns, in the trivial case.  Pseudo-code:
>
>    function recurse (target)
>    {
>      if (isTrivial(target)) { return trivialValue; }
>
>      for (var property in target)
>      {
>        recurse(target[property]);
>      }
>
>      return nonTrivialValue;
>    }

I'll try to make a changes as you suggested above.

>>       }
>>
>>       if (typeof text !== 'undefined') {
>
> Again an unnecessary strict comparison.

JSLint case.

>>           if (is_block_style(node) === 'block') {
>>               node.parentNode.insertBefore(document.createElement('br'),
>> node.nextSibling);
>>           }
>
> Why?  Usually it is misuse of the “br” element to use it as replacement for
> a block delimiter.  Also, “display: block” does not necessarily mean that
> there is a paragraph.  Consider, for example, positioned or floating block
> elements.

Yes, I now saw the method getKomputedStyle from MyLibrary. 
Unfortunately, it's quite complicated :/ I do not know about JSX similar 
method.

>>           //node.parentNode.replaceChild(document.createTextNode(text),
>> node);
>>       }
>>       return text;
>> };
>
> And BTW, “doesn't work (yet)” is not a problem description.

I'll try to be more strictly next time.

Will apply now some changes, as you mentioned.

-- 
Cezary Tomczyk
http://www.ctomczyk.pl/

Back to comp.lang.javascript | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

Clean up HTML content Cezary Tomczyk <cezary.tomczyk@gmail.com> - 2013-03-05 08:48 +0100
  Re: Clean up HTML content Thomas 'PointedEars' Lahn <PointedEars@web.de> - 2013-03-05 17:55 +0100
    Re: Clean up HTML content Cezary Tomczyk <cezary.tomczyk@gmail.com> - 2013-03-05 20:25 +0100
      Re: Clean up HTML content Thomas 'PointedEars' Lahn <PointedEars@web.de> - 2013-03-06 00:03 +0100
        Re: Clean up HTML content Thomas 'PointedEars' Lahn <PointedEars@web.de> - 2013-03-06 00:08 +0100
          Re: Clean up HTML content Dr J R Stockton <reply1300@merlyn.demon.co.uk.invalid> - 2013-03-06 22:06 +0000
            Re: Clean up HTML content Eric Bednarz <bednarz@fahr-zur-hoelle.org> - 2013-03-10 01:14 +0100
        Re: Clean up HTML content Cezary Tomczyk <cezary.tomczyk@gmail.com> - 2013-03-06 22:02 +0100
          Re: Clean up HTML content Thomas 'PointedEars' Lahn <PointedEars@web.de> - 2013-03-06 23:45 +0100
            Re: Clean up HTML content Cezary Tomczyk <cezary.tomczyk@gmail.com> - 2013-03-09 11:29 +0100
              Re: Clean up HTML content Thomas 'PointedEars' Lahn <PointedEars@web.de> - 2013-03-09 16:11 +0100
  Re: Clean up HTML content Danny <dann90038@gmail.com> - 2013-03-07 14:28 -0800
    Re: Clean up HTML content Cezary Tomczyk <cezary.tomczyk@gmail.com> - 2013-03-09 11:37 +0100
      Re: Clean up HTML content dann90038@gmail.com - 2013-03-09 11:44 -0800

csiph-web