[webkit-reviews] review requested: [Bug 17897] Not Rendering Images Imported from XHTML Document : [Attachment 25079] Another round: Remove the boolean and

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at pleyo.com> has asked Darin Adler <darin at apple.com>
for review:
Bug 17897: Not Rendering Images Imported from XHTML Document
https://bugs.webkit.org/show_bug.cgi?id=17897

Attachment 25079: Another round: Remove the boolean and 
https://bugs.webkit.org/attachment.cgi?id=25079&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at pleyo.com>
> The reason I don't like boolean arguments to functions like this is it's so
> hard to understand them at the call site. It would be a lot nicer if that
> argument had a name like TryAgain instead of just being true, or if this was
a
> separate function with a sensible name, even if it only ends up being a
boolean
> argument anyway.

> After reading the logic carefully, I realize that you only need to store the
> old URL in the case where the load failed. I think we could change this to >
work
> that way. Then you could have m_failedLoadURL instead of m_sourceURL and not
> bother with m_hadError at all. I think the logic would stay simple and clear
> in
> that case.

I have given it some thought and rewritten the error handling part to remove
the m_hadError bool (as you were rightly pointing out). I also used this
iteration to remove the boolean and add a method
updateFromElementIgnoringPreviousError. This should address the two previous
comments.