Adding <img> support is trivial, as there already exists a common ImageLoader class, and SVG/HTMLImage{Element/Loader} already share as much code as possible. Only difference for WML is that there exists a second source attribute called 'localsrc' - that takes precedence over the 'src' attribute. If loading 'localsrc' fails, 'src' should be tried instead - crazy :-)
Created attachment 25967 [details] Initial patch Patch contains a sorted version of WebCore.xcodeproj/project.pbxproj, that enlarges the patch a bit.
Created attachment 25968 [details] LayoutTests
Comment on attachment 25968 [details] LayoutTests This looks sane.
Created attachment 25969 [details] Updated patch Incorporated Holgers comments.
Comment on attachment 25969 [details] Updated patch > Index: WebCore.xcodeproj/project.pbxproj > =================================================================== > --- WebCore.xcodeproj/project.pbxproj (revision 39224) > +++ WebCore.xcodeproj/project.pbxproj (working copy) changing the order is okay according to Niko. > Index: rendering/HitTestResult.cpp > =================================================================== > --- rendering/HitTestResult.cpp (revision 39224) > +++ rendering/HitTestResult.cpp (working copy) > if (!m_innerNonSharedNode) > return String(); > - > + > if (m_innerNonSharedNode->hasTagName(imgTag)) { > HTMLImageElement* image = static_cast<HTMLImageElement*>(m_innerNonSharedNode.get()); > return displayString(image->alt(), m_innerNonSharedNode.get()); > } > - > + > if (m_innerNonSharedNode->hasTagName(inputTag)) { > HTMLInputElement* input = static_cast<HTMLInputElement*>(m_innerNonSharedNode.get()); > return displayString(input->alt(), m_innerNonSharedNode.get()); > } > - > + please land without these whitespace changes. rs=me if you want to fix the bogus space. > @@ -232,11 +240,14 @@ KURL HitTestResult::absoluteImageURL() c > AtomicString urlString; > if (m_innerNonSharedNode->hasTagName(imgTag) || m_innerNonSharedNode->hasTagName(inputTag)) > urlString = static_cast<Element*>(m_innerNonSharedNode.get())->getAttribute(srcAttr); > + else if (m_innerNonSharedNode->hasTagName(embedTag) > #if ENABLE(SVG) > - else if (m_innerNonSharedNode->hasTagName(SVGNames::imageTag)) > - urlString = static_cast<Element*>(m_innerNonSharedNode.get())->getAttribute(XLinkNames::hrefAttr); > + || m_innerNonSharedNode->hasTagName(SVGNames::imageTag) > +#endif > +#if ENABLE(WML) > + || m_innerNonSharedNode->hasTagName(WMLNames::imgTag) > #endif > - else if (m_innerNonSharedNode->hasTagName(embedTag) || m_innerNonSharedNode->hasTagName(objectTag)) { > + || m_innerNonSharedNode->hasTagName(objectTag)) { yes, I checked, SVGImageElement returns XLinkNames::hrefAttr for imageSourceAttributeName...so it could be correct. > + imageObj->setCachedImage(m_imageLoader.image()); > + > + // If we have no image at all because we have no src attribute, set > + // image height and width for the alt text instead. > + if (!m_imageLoader.image() && !imageObj->cachedImage()) > + imageObj->setImageSizeForAltText(); I see this is from HTMLImageElement. But after the above setCachedImage the following assert should be true. ASSERT(imageObj->cachedImage() == m_imageLoader.image())? If that is the case checking for m_imageLoader.image() or imageObj->cachedImage would be enough... but this can be cleaned up later. > + // WML doesn't fire any events. > + if (haveFiredLoadEvent()) > + return; > + > + setHaveFiredLoadEvent(true); hehe... > +void WMLImageLoader::notifyFinished(CachedResource* image) > +{ yes, this should fallback to src when the loading failed and there is a HTMLNames::srcAttr on the WMLImageElement. Looks good.
Thanks, Holger. Fixed your comments. Except the one affecting HTMLImageLoader, it should be fixed alltogether in a seperated patch. Landed in r39225.