- Revision
- 224973
- Author
- [email protected]
- Date
- 2017-11-17 10:40:09 -0800 (Fri, 17 Nov 2017)
Log Message
SVG scrolling anchor should be reset if the fragmentIdentifier does not exist or is not provided
https://bugs.webkit.org/show_bug.cgi?id=176577
Patch by Said Abou-Hallawa <[email protected]> on 2017-11-17
Reviewed by Simon Fraser.
Source/WebCore:
Because the SVGImage can be cached only once but used multiple times with
different fragmentIdentifiers, SVGImage has to call FrameView::scrollToFragment()
before the image is displayed. If the fragmentIdentifier is not provided
in the URL or it does not exist in the SVGImage, FrameView::scrollToFragment()
has to reset the scrolling anchor of the SVG as if it was not displayed before.
We do not want the previous scrolling anchor to be used when the FrameView
of SVGImage can't scroll to the current fragmentIdentifier for any reason.
Test: http/tests/svg/svg-fragment-url-special-cases.html
* page/FrameView.cpp:
(WebCore::FrameView::scrollToFragment):
(WebCore::FrameView::scrollToAnchor):
(WebCore::FrameView::resetScrollAnchor):
* page/FrameView.h:
* platform/URL.cpp:
(WebCore::URL::fragmentIdentifier const): Call hasFragmentIdentifier()
instead of repeating the same condition.
(WebCore::decodeURLEscapeSequences):
* svg/SVGSVGElement.cpp:
(WebCore::SVGSVGElement::findViewAnchor const):
(WebCore::SVGSVGElement::findRootAnchor const):
(WebCore::SVGSVGElement::scrollToAnchor): We want to know whether the SVG
root element could scroll to the fragmentIdentifier or not. If it could not,
FrameView::scrollToAnchor() can still try one last time to do the scrolling
only if anchorElement is not nullptr.
(WebCore::SVGSVGElement::resetScrollAnchor): Reset the FrameView scrolling
state to its initial value.
* svg/SVGSVGElement.h:
LayoutTests:
* http/tests/svg/svg-fragment-url-special-cases-expected.html: Added.
* http/tests/svg/svg-fragment-url-special-cases.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (224972 => 224973)
--- trunk/LayoutTests/ChangeLog 2017-11-17 18:38:32 UTC (rev 224972)
+++ trunk/LayoutTests/ChangeLog 2017-11-17 18:40:09 UTC (rev 224973)
@@ -1,3 +1,13 @@
+2017-11-17 Said Abou-Hallawa <[email protected]>
+
+ SVG scrolling anchor should be reset if the fragmentIdentifier does not exist or is not provided
+ https://bugs.webkit.org/show_bug.cgi?id=176577
+
+ Reviewed by Simon Fraser.
+
+ * http/tests/svg/svg-fragment-url-special-cases-expected.html: Added.
+ * http/tests/svg/svg-fragment-url-special-cases.html: Added.
+
2017-11-16 Antoine Quint <[email protected]>
[Web Animations] Force a stacking context during animations that animate properties that will force a stacking context
Added: trunk/LayoutTests/http/tests/svg/svg-fragment-url-special-cases-expected.html (0 => 224973)
--- trunk/LayoutTests/http/tests/svg/svg-fragment-url-special-cases-expected.html (rev 0)
+++ trunk/LayoutTests/http/tests/svg/svg-fragment-url-special-cases-expected.html 2017-11-17 18:40:09 UTC (rev 224973)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ img {
+ width: 100px;
+ height: 100px;
+ }
+ span {
+ display: inline-block;
+ width: 100px;
+ height: 100px;
+ background-color: green;
+ }
+ </style>
+</head>
+<body>
+ <div id="group-1">
+ <p>Image URL with no fragmentIdentifier:</p>
+ <img src=""
+ <img src=""
+ </div>
+ <div id="group-2">
+ <p>Image URL with trailing spaces after the fragmentIdentifier:</p>
+ <span></span>
+ <span></span>
+ <span></span>
+ </div>
+ <div id="group-3">
+ <p>Image URL with non-existent fragmentIdentifier:</p>
+ <img src=""
+ <img src=""
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/svg/svg-fragment-url-special-cases.html (0 => 224973)
--- trunk/LayoutTests/http/tests/svg/svg-fragment-url-special-cases.html (rev 0)
+++ trunk/LayoutTests/http/tests/svg/svg-fragment-url-special-cases.html 2017-11-17 18:40:09 UTC (rev 224973)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ img {
+ width: 100px;
+ height: 100px;
+ }
+ </style>
+</head>
+<body>
+ <div id="group-1">
+ <p>Image URL with no fragmentIdentifier:</p>
+ <img src=""
+ <img src=""
+ <img src=""
+ </div>
+ <div id="group-2">
+ <p>Image URL with trailing spaces after the fragmentIdentifier:</p>
+ <img src="" ">
+ <img src="" ">
+ <img src="" ">
+ </div>
+ <div id="group-3">
+ <p>Image URL with non-existent fragmentIdentifier:</p>
+ <img src=""
+ <img src=""
+ <img src=""
+ </div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (224972 => 224973)
--- trunk/Source/WebCore/ChangeLog 2017-11-17 18:38:32 UTC (rev 224972)
+++ trunk/Source/WebCore/ChangeLog 2017-11-17 18:40:09 UTC (rev 224973)
@@ -1,3 +1,40 @@
+2017-11-17 Said Abou-Hallawa <[email protected]>
+
+ SVG scrolling anchor should be reset if the fragmentIdentifier does not exist or is not provided
+ https://bugs.webkit.org/show_bug.cgi?id=176577
+
+ Reviewed by Simon Fraser.
+
+ Because the SVGImage can be cached only once but used multiple times with
+ different fragmentIdentifiers, SVGImage has to call FrameView::scrollToFragment()
+ before the image is displayed. If the fragmentIdentifier is not provided
+ in the URL or it does not exist in the SVGImage, FrameView::scrollToFragment()
+ has to reset the scrolling anchor of the SVG as if it was not displayed before.
+ We do not want the previous scrolling anchor to be used when the FrameView
+ of SVGImage can't scroll to the current fragmentIdentifier for any reason.
+
+ Test: http/tests/svg/svg-fragment-url-special-cases.html
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::scrollToFragment):
+ (WebCore::FrameView::scrollToAnchor):
+ (WebCore::FrameView::resetScrollAnchor):
+ * page/FrameView.h:
+ * platform/URL.cpp:
+ (WebCore::URL::fragmentIdentifier const): Call hasFragmentIdentifier()
+ instead of repeating the same condition.
+ (WebCore::decodeURLEscapeSequences):
+ * svg/SVGSVGElement.cpp:
+ (WebCore::SVGSVGElement::findViewAnchor const):
+ (WebCore::SVGSVGElement::findRootAnchor const):
+ (WebCore::SVGSVGElement::scrollToAnchor): We want to know whether the SVG
+ root element could scroll to the fragmentIdentifier or not. If it could not,
+ FrameView::scrollToAnchor() can still try one last time to do the scrolling
+ only if anchorElement is not nullptr.
+ (WebCore::SVGSVGElement::resetScrollAnchor): Reset the FrameView scrolling
+ state to its initial value.
+ * svg/SVGSVGElement.h:
+
2017-11-17 Youenn Fablet <[email protected]>
ServiceWorker intercepted FetchRequest should have their referrer set appropriately.
Modified: trunk/Source/WebCore/page/FrameView.cpp (224972 => 224973)
--- trunk/Source/WebCore/page/FrameView.cpp 2017-11-17 18:38:32 UTC (rev 224972)
+++ trunk/Source/WebCore/page/FrameView.cpp 2017-11-17 18:40:09 UTC (rev 224973)
@@ -2086,28 +2086,26 @@
bool FrameView::scrollToFragment(const URL& url)
{
- // If our URL has no ref, then we have no place we need to jump to.
- // OTOH If CSS target was set previously, we want to set it to 0, recalc
- // and possibly repaint because :target pseudo class may have been
- // set (see bug 11321).
- if (!url.hasFragmentIdentifier()) {
- frame().document()->setCSSTarget(nullptr);
- return false;
- }
-
String fragmentIdentifier = url.fragmentIdentifier();
if (scrollToAnchor(fragmentIdentifier))
return true;
// Try again after decoding the ref, based on the document's encoding.
- if (TextResourceDecoder* decoder = frame().document()->decoder())
- return scrollToAnchor(decodeURLEscapeSequences(fragmentIdentifier, decoder->encoding()));
+ if (TextResourceDecoder* decoder = frame().document()->decoder()) {
+ if (scrollToAnchor(decodeURLEscapeSequences(fragmentIdentifier, decoder->encoding())))
+ return true;
+ }
+ resetScrollAnchor();
return false;
}
-bool FrameView::scrollToAnchor(const String& name)
+bool FrameView::scrollToAnchor(const String& fragmentIdentifier)
{
+ // If our URL has no ref, then we have no place we need to jump to.
+ if (fragmentIdentifier.isNull())
+ return false;
+
ASSERT(frame().document());
auto& document = *frame().document();
@@ -2118,23 +2116,26 @@
document.setGotoAnchorNeededAfterStylesheetsLoad(false);
- Element* anchorElement = document.findAnchor(name);
+ Element* anchorElement = document.findAnchor(fragmentIdentifier);
// Setting to null will clear the current target.
document.setCSSTarget(anchorElement);
if (is<SVGDocument>(document)) {
+ if (fragmentIdentifier.isEmpty())
+ return false;
if (auto rootElement = SVGDocument::rootElement(document)) {
- rootElement->scrollToAnchor(name, anchorElement);
+ if (rootElement->scrollToFragment(fragmentIdentifier))
+ return true;
+ // If SVG failed to scrollToAnchor() and anchorElement is null, no other scrolling will be possible.
if (!anchorElement)
- return true;
+ return false;
}
+ } else if (!anchorElement && !(fragmentIdentifier.isEmpty() || equalLettersIgnoringASCIICase(fragmentIdentifier, "top"))) {
+ // Implement the rule that "" and "top" both mean top of page as in other browsers.
+ return false;
}
- // Implement the rule that "" and "top" both mean top of page as in other browsers.
- if (!anchorElement && !(name.isEmpty() || equalLettersIgnoringASCIICase(name, "top")))
- return false;
-
ContainerNode* scrollPositionAnchor = anchorElement;
if (!scrollPositionAnchor)
scrollPositionAnchor = frame().document();
@@ -2192,6 +2193,26 @@
ScrollView::setScrollPosition(scrollPosition);
}
+void FrameView::resetScrollAnchor()
+{
+ ASSERT(frame().document());
+ auto& document = *frame().document();
+
+ // If CSS target was set previously, we want to set it to 0, recalc
+ // and possibly repaint because :target pseudo class may have been
+ // set (see bug 11321).
+ document.setCSSTarget(nullptr);
+
+ if (is<SVGDocument>(document)) {
+ if (auto rootElement = SVGDocument::rootElement(document)) {
+ // We need to update the layout before resetScrollAnchor(), otherwise we
+ // could really mess things up if resetting the anchor comes at a bad moment.
+ document.updateStyleIfNeeded();
+ rootElement->resetScrollAnchor();
+ }
+ }
+}
+
void FrameView::contentsResized()
{
// For non-delegated scrolling, updateTiledBackingAdaptiveSizing() is called via addedOrRemovedScrollbar() which occurs less often.
Modified: trunk/Source/WebCore/page/FrameView.h (224972 => 224973)
--- trunk/Source/WebCore/page/FrameView.h 2017-11-17 18:38:32 UTC (rev 224972)
+++ trunk/Source/WebCore/page/FrameView.h 2017-11-17 18:40:09 UTC (rev 224973)
@@ -742,6 +742,7 @@
void scrollPositionChanged(const ScrollPosition& oldPosition, const ScrollPosition& newPosition);
void scrollableAreaSetChanged();
void sendScrollEvent();
+ void resetScrollAnchor();
bool hasCustomScrollbars() const;
Modified: trunk/Source/WebCore/platform/URL.cpp (224972 => 224973)
--- trunk/Source/WebCore/platform/URL.cpp 2017-11-17 18:38:32 UTC (rev 224972)
+++ trunk/Source/WebCore/platform/URL.cpp 2017-11-17 18:40:09 UTC (rev 224973)
@@ -224,7 +224,7 @@
String URL::fragmentIdentifier() const
{
- if (!m_isValid || m_queryEnd == m_string.length())
+ if (!hasFragmentIdentifier())
return String();
return m_string.substring(m_queryEnd + 1);
@@ -674,11 +674,15 @@
String decodeURLEscapeSequences(const String& string)
{
+ if (string.isEmpty())
+ return string;
return decodeEscapeSequences<URLEscapeSequence>(string, UTF8Encoding());
}
String decodeURLEscapeSequences(const String& string, const TextEncoding& encoding)
{
+ if (string.isEmpty())
+ return string;
return decodeEscapeSequences<URLEscapeSequence>(string, encoding);
}
Modified: trunk/Source/WebCore/svg/SVGSVGElement.cpp (224972 => 224973)
--- trunk/Source/WebCore/svg/SVGSVGElement.cpp 2017-11-17 18:38:32 UTC (rev 224972)
+++ trunk/Source/WebCore/svg/SVGSVGElement.cpp 2017-11-17 18:40:09 UTC (rev 224973)
@@ -635,8 +635,27 @@
return transform;
}
-void SVGSVGElement::scrollToAnchor(const String& fragmentIdentifier, Element* anchorNode)
+SVGViewElement* SVGSVGElement::findViewAnchor(const String& fragmentIdentifier) const
{
+ auto* anchorElement = document().findAnchor(fragmentIdentifier);
+ return is<SVGViewElement>(anchorElement) ? downcast<SVGViewElement>(anchorElement): nullptr;
+}
+
+SVGSVGElement* SVGSVGElement::findRootAnchor(const SVGViewElement* viewElement) const
+{
+ auto* viewportElement = SVGLocatable::nearestViewportElement(viewElement);
+ return is<SVGSVGElement>(viewportElement) ? downcast<SVGSVGElement>(viewportElement) : nullptr;
+}
+
+SVGSVGElement* SVGSVGElement::findRootAnchor(const String& fragmentIdentifier) const
+{
+ if (auto* viewElement = findViewAnchor(fragmentIdentifier))
+ return findRootAnchor(viewElement);
+ return nullptr;
+}
+
+bool SVGSVGElement::scrollToFragment(const String& fragmentIdentifier)
+{
auto renderer = this->renderer();
auto view = m_viewSpec;
if (view)
@@ -649,7 +668,7 @@
// FIXME: XPointer references are ignored (https://bugs.webkit.org/show_bug.cgi?id=17491)
if (renderer && hadUseCurrentView)
RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer);
- return;
+ return false;
}
if (fragmentIdentifier.startsWith("svgView(")) {
@@ -661,7 +680,7 @@
view->reset();
if (renderer && (hadUseCurrentView || m_useCurrentView))
RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer);
- return;
+ return m_useCurrentView;
}
// Spec: If the SVG fragment identifier addresses a "view" element within an SVG document (e.g., MyDrawing.svg#MyView
@@ -668,22 +687,44 @@
// or MyDrawing.svg#xpointer(id('MyView'))) then the closest ancestor "svg" element is displayed in the viewport.
// Any view specification attributes included on the given "view" element override the corresponding view specification
// attributes on the closest ancestor "svg" element.
- if (is<SVGViewElement>(anchorNode)) {
- auto& viewElement = downcast<SVGViewElement>(*anchorNode);
- auto viewportElement = makeRefPtr(SVGLocatable::nearestViewportElement(&viewElement));
- if (is<SVGSVGElement>(viewportElement)) {
- auto& element = downcast<SVGSVGElement>(*viewportElement);
- element.inheritViewAttributes(viewElement);
- if (auto* renderer = element.renderer())
+ if (auto* viewElement = findViewAnchor(fragmentIdentifier)) {
+ if (auto* rootElement = findRootAnchor(viewElement)) {
+ rootElement->inheritViewAttributes(*viewElement);
+ if (auto* renderer = rootElement->renderer())
RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer);
+ m_currentViewFragmentIdentifier = fragmentIdentifier;
+ return true;
}
- return;
}
// FIXME: We need to decide which <svg> to focus on, and zoom to it.
// FIXME: We need to actually "highlight" the viewTarget(s).
+ return false;
}
+void SVGSVGElement::resetScrollAnchor()
+{
+ if (!m_useCurrentView && m_currentViewFragmentIdentifier.isEmpty())
+ return;
+
+ if (m_viewSpec)
+ m_viewSpec->reset();
+
+ if (!m_currentViewFragmentIdentifier.isEmpty()) {
+ if (auto* rootElement = findRootAnchor(m_currentViewFragmentIdentifier)) {
+ SVGViewSpec& view = rootElement->currentView();
+ view.setViewBoxBaseValue(viewBox());
+ view.setPreserveAspectRatioBaseValue(preserveAspectRatioBaseValue());
+ view.setZoomAndPanBaseValue(zoomAndPan());
+ m_currentViewFragmentIdentifier = { };
+ }
+ }
+
+ m_useCurrentView = false;
+ if (renderer())
+ RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer());
+}
+
void SVGSVGElement::inheritViewAttributes(const SVGViewElement& viewElement)
{
SVGViewSpec& view = currentView();
Modified: trunk/Source/WebCore/svg/SVGSVGElement.h (224972 => 224973)
--- trunk/Source/WebCore/svg/SVGSVGElement.h 2017-11-17 18:38:32 UTC (rev 224972)
+++ trunk/Source/WebCore/svg/SVGSVGElement.h 2017-11-17 18:40:09 UTC (rev 224973)
@@ -113,7 +113,8 @@
public:
static Ref<SVGSVGElement> create(const QualifiedName&, Document&);
static Ref<SVGSVGElement> create(Document&);
- void scrollToAnchor(const String& fragmentIdentifier, Element* anchor);
+ bool scrollToFragment(const String& fragmentIdentifier);
+ void resetScrollAnchor();
using SVGGraphicsElement::ref;
using SVGGraphicsElement::deref;
@@ -155,11 +156,16 @@
void inheritViewAttributes(const SVGViewElement&);
Ref<NodeList> collectIntersectionOrEnclosureList(SVGRect&, SVGElement*, bool (*checkFunction)(SVGElement&, SVGRect&));
+ SVGViewElement* findViewAnchor(const String& fragmentIdentifier) const;
+ SVGSVGElement* findRootAnchor(const SVGViewElement*) const;
+ SVGSVGElement* findRootAnchor(const String&) const;
+
bool m_useCurrentView { false };
SVGZoomAndPanType m_zoomAndPan { SVGZoomAndPanMagnify };
Ref<SMILTimeContainer> m_timeContainer;
FloatPoint m_currentTranslate;
RefPtr<SVGViewSpec> m_viewSpec;
+ String m_currentViewFragmentIdentifier;
};
inline bool SVGSVGElement::useCurrentView() const
