The original submission is mistaken, I didn't understand the bug until I debugged it, so the title is misleading. Do not confuse with #29322, it is a different bug.

Proper explanation:
The problem only happens when a tag has a style attribute, this attribute is not the last one and the style has any urls (that's why it works with absolute urls, they are not modified) to be converted with --convert-links.

So, <a style="background-image: url('foo.gif');" href="somepage.htm">text</a> is bugged

and <a href="somepage.htm" style="background-image: url('foo.gif');">text</a> is not

Problem:

Tag handlers add urlpos structures to the tail of urlpos list (tail pointer in ctx structure). That is done when they call append_url.

After the handler is called, check_style_attr checks for a style attribute in all the tags, and if found, more urlpos structures are added to the tail of the list.

If that happens, and any urlpos structures had been added in its handler, the list isn't ordered any more. That is a problem, it must be ordered when writing the output file if you are using convert-links.

Fix:

You can't really guess where the urlpos structures found in the style attribute should be inserted in the list referenced by pointers head and tail in ctx structure.

Not without parsing all the tags again, and that's already been done by their handlers. check_style_attr is important because if you remove it, style would be ignored for tags not in known_tags[] (well, not in interesting_tags), like div.

Normal tag handlers must be changed so they check for a style attribute if the tag allows it. Then you could somehow remember the position in the list and pass it to check_attributes, but that would be a mess. As you must find the style position, We can just call get_urls, the same thing check_style_attr is doing.
This way urls in style attributes are inserted in order.

Code is needed to avoid calling check_style_attributes in those cases where the handler already does the work. An easy but hackish way of doing it is storing the ctx->tail pointer before calling the handler. Because handlers only add
urls to the tail, if it doesn't change, then no urls have been added, and this bug wouldn't be a problem. This is easy to implement, but if you change the way handlers work, if they inserted urls at random positions instead of appending them to the list, THIS CODE WOULD BREAK. So I'm not doing it.

Another fix would be forcing all the handlers to check for a style attribute. Then you would call check_style_attributes just for those tags not in interesting_tags (the ones that don't have a handler). The only handlers that could avoid checking for style would be the ones only used by tags that support the attribute AND DON'T add URLs to the list. There are no such handlers (yet).

Only meta and base handlers are not checking for this, because they don't support style. All the others do, and also add URLs. I'm explaining this because I was going to implement an array to add tags you want to check for style and don't want to do it in the handler. However, it adds complexity and wouldn't be used much, certainly not now. So again, I didn't do it.

So, ALL the handlers of tags that support style, MUST check for it. No exception.


An alternative to avoid changing the handlers would be to store the number of attributes with URLs a tag has. Then check_style_attr could find the style attribute and know where it should be inserted, counting the number of those attributes in all the previous tags. Once known that it's just a matter of finding the position in the urlpos linked list.

In my opinion, this is a lot more inefficient and complicated, I see no easy fix to avoid forcing you to do the check in the handler. Anyway I don't think this is a problem. I tried to avoid touching the handlers just because I wanted to fix it without changing too many things.


To test the patch: wget --convert-links http://pastehtml.com/view/5txt9l4.html

I found this while trying to download a mediawiki page with the Monobook skin. Wikipedia also uses that skin, but it works because they are using a full url to point to the background. However it can be changed in the config file, giving you that problem with wget. Anyway, syntax is valid, so many webpages will be using it, not just mediawiki.