Summary: | Add WML <img> element support | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikolas Zimmermann <zimmermann> | ||||||||
Component: | XML | Assignee: | Nikolas Zimmermann <zimmermann> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | ||||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.5 | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 20393 | ||||||||||
Attachments: |
|
Description
Nikolas Zimmermann
2008-12-11 16:23:07 PST
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. |