Diff
Modified: branches/safari-600.1-branch/Source/WebCore/ChangeLog (172552 => 172553)
--- branches/safari-600.1-branch/Source/WebCore/ChangeLog 2014-08-14 03:19:02 UTC (rev 172552)
+++ branches/safari-600.1-branch/Source/WebCore/ChangeLog 2014-08-14 03:38:54 UTC (rev 172553)
@@ -1,3 +1,26 @@
+2014-08-13 Lucas Forschler <lforsch...@apple.com>
+
+ Merge r172383
+
+ 2014-08-10 Tim Horton <timothy_hor...@apple.com>
+
+ Yelp phone number highlights often disappear
+ https://bugs.webkit.org/show_bug.cgi?id=135789
+ <rdar://problem/17971057>
+
+ Reviewed by Brady Eidson.
+
+ * editing/Editor.cpp:
+ (WebCore::Editor::scanSelectionForTelephoneNumbers):
+ (WebCore::Editor::clearDataDetectedTelephoneNumbers): Deleted.
+ * editing/Editor.h:
+ (WebCore::Editor::detectedTelephoneNumberRanges):
+ * page/EditorClient.h:
+ (WebCore::EditorClient::selectedTelephoneNumberRangesChanged):
+ Cache and expose detected telephone number ranges on Editor.
+ Change selectedTelephoneNumberRangesChanged to take no arguments; it's
+ just a notification now.
+
2014-08-12 Lucas Forschler <lforsch...@apple.com>
Merge r172471
Modified: branches/safari-600.1-branch/Source/WebCore/editing/Editor.cpp (172552 => 172553)
--- branches/safari-600.1-branch/Source/WebCore/editing/Editor.cpp 2014-08-14 03:19:02 UTC (rev 172552)
+++ branches/safari-600.1-branch/Source/WebCore/editing/Editor.cpp 2014-08-14 03:38:54 UTC (rev 172553)
@@ -3362,11 +3362,13 @@
if (!shouldDetectTelephoneNumbers() || !client())
return;
+ m_detectedTelephoneNumberRanges.clear();
+
Vector<RefPtr<Range>> markedRanges;
FrameSelection& frameSelection = m_frame.selection();
if (!frameSelection.isRange()) {
- client()->selectedTelephoneNumberRangesChanged(markedRanges);
+ client()->selectedTelephoneNumberRangesChanged();
return;
}
RefPtr<Range> selectedRange = frameSelection.toNormalizedRange();
@@ -3394,20 +3396,19 @@
RefPtr<Range> extendedRange = extendedSelection.toNormalizedRange();
if (!extendedRange) {
- client()->selectedTelephoneNumberRangesChanged(markedRanges);
+ client()->selectedTelephoneNumberRangesChanged();
return;
}
scanRangeForTelephoneNumbers(*extendedRange, extendedRange->text(), markedRanges);
// Only consider ranges with a detected telephone number if they overlap with the actual selection range.
- Vector<RefPtr<Range>> markedRangesIntersectingSelection;
for (auto& range : markedRanges) {
if (rangesOverlap(range.get(), selectedRange.get()))
- markedRangesIntersectingSelection.append(range);
+ m_detectedTelephoneNumberRanges.append(range);
}
- client()->selectedTelephoneNumberRangesChanged(markedRangesIntersectingSelection);
+ client()->selectedTelephoneNumberRangesChanged();
}
void Editor::scanRangeForTelephoneNumbers(Range& range, const StringView& stringView, Vector<RefPtr<Range>>& markedRanges)
@@ -3448,13 +3449,6 @@
}
}
-void Editor::clearDataDetectedTelephoneNumbers()
-{
- document().markers().removeMarkers(DocumentMarker::TelephoneNumber);
-
- // FIXME: Do other UI cleanup here once we have other UI.
-}
-
#endif // ENABLE(TELEPHONE_NUMBER_DETECTION) && !PLATFORM(IOS)
void Editor::updateEditorUINowIfScheduled()
Modified: branches/safari-600.1-branch/Source/WebCore/editing/Editor.h (172552 => 172553)
--- branches/safari-600.1-branch/Source/WebCore/editing/Editor.h 2014-08-14 03:19:02 UTC (rev 172552)
+++ branches/safari-600.1-branch/Source/WebCore/editing/Editor.h 2014-08-14 03:38:54 UTC (rev 172553)
@@ -450,6 +450,7 @@
#if ENABLE(TELEPHONE_NUMBER_DETECTION) && !PLATFORM(IOS)
void scanSelectionForTelephoneNumbers();
+ const Vector<RefPtr<Range>>& detectedTelephoneNumberRanges() const { return m_detectedTelephoneNumberRanges; }
#endif
private:
@@ -521,9 +522,9 @@
bool shouldDetectTelephoneNumbers();
void scanSelectionForTelephoneNumbers(Timer<Editor>&);
void scanRangeForTelephoneNumbers(Range&, const StringView&, Vector<RefPtr<Range>>& markedRanges);
- void clearDataDetectedTelephoneNumbers();
Timer<Editor> m_telephoneNumberDetectionUpdateTimer;
+ Vector<RefPtr<Range>> m_detectedTelephoneNumberRanges;
#endif
};
Modified: branches/safari-600.1-branch/Source/WebCore/page/EditorClient.h (172552 => 172553)
--- branches/safari-600.1-branch/Source/WebCore/page/EditorClient.h 2014-08-14 03:19:02 UTC (rev 172552)
+++ branches/safari-600.1-branch/Source/WebCore/page/EditorClient.h 2014-08-14 03:38:54 UTC (rev 172553)
@@ -182,7 +182,7 @@
virtual void setInputMethodState(bool enabled) = 0;
#if ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
- virtual void selectedTelephoneNumberRangesChanged(const Vector<RefPtr<Range>>&) { }
+ virtual void selectedTelephoneNumberRangesChanged() { }
virtual void selectionRectsDidChange(const Vector<LayoutRect>&, const Vector<GapRects>&, bool) { }
#endif
Modified: branches/safari-600.1-branch/Source/WebKit2/ChangeLog (172552 => 172553)
--- branches/safari-600.1-branch/Source/WebKit2/ChangeLog 2014-08-14 03:19:02 UTC (rev 172552)
+++ branches/safari-600.1-branch/Source/WebKit2/ChangeLog 2014-08-14 03:38:54 UTC (rev 172553)
@@ -1,5 +1,47 @@
2014-08-13 Lucas Forschler <lforsch...@apple.com>
+ Merge r172383
+
+ 2014-08-10 Tim Horton <timothy_hor...@apple.com>
+
+ Yelp phone number highlights often disappear
+ https://bugs.webkit.org/show_bug.cgi?id=135789
+ <rdar://problem/17971057>
+
+ Reviewed by Brady Eidson.
+
+ Since selectedTelephoneNumberRangesChanged doesn't provide an associated
+ Frame, an incoming selectedTelephoneNumberRangesChanged from a subframe
+ would overwrite ServicesOverlayController's cached (and potentially active)
+ telephone number highlights.
+
+ This happens a lot on Yelp, because they have many subframes which are
+ doing layout on a regular basis.
+
+ * WebProcess/WebCoreSupport/WebEditorClient.cpp:
+ (WebKit::WebEditorClient::selectedTelephoneNumberRangesChanged):
+ * WebProcess/WebCoreSupport/WebEditorClient.h:
+ Adjust to the new (lack of) arguments.
+
+ * WebProcess/WebPage/ServicesOverlayController.h:
+ * WebProcess/WebPage/mac/ServicesOverlayController.mm:
+ (WebKit::ServicesOverlayController::selectedTelephoneNumberRangesChanged):
+ Adjust logging; we can revisit it later.
+
+ (WebKit::ServicesOverlayController::buildPhoneNumberHighlights):
+ When building phone number highlights, walk the Frame tree and retrieve
+ them from all of the Editors.
+
+ (WebKit::ServicesOverlayController::didRebuildPotentialHighlights):
+ (WebKit::ServicesOverlayController::telephoneNumberRangesForFocusedFrame):
+ (WebKit::ServicesOverlayController::determineActiveHighlight):
+ (WebKit::ServicesOverlayController::handleClick):
+ Retrieve the detected telephone number ranges from the focused frame
+ when combining telephone numbers with selection services.
+ This ensures that we don't show combined phone number highlights from other frames.
+
+2014-08-13 Lucas Forschler <lforsch...@apple.com>
+
Merge r172382
2014-08-10 Tim Horton <timothy_hor...@apple.com>
Modified: branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp (172552 => 172553)
--- branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp 2014-08-14 03:19:02 UTC (rev 172552)
+++ branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp 2014-08-14 03:38:54 UTC (rev 172553)
@@ -532,10 +532,10 @@
}
#if ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
-void WebEditorClient::selectedTelephoneNumberRangesChanged(const Vector<RefPtr<Range>>& ranges)
+void WebEditorClient::selectedTelephoneNumberRangesChanged()
{
#if PLATFORM(MAC)
- m_page->servicesOverlayController().selectedTelephoneNumberRangesChanged(ranges);
+ m_page->servicesOverlayController().selectedTelephoneNumberRangesChanged();
#endif
}
void WebEditorClient::selectionRectsDidChange(const Vector<LayoutRect>& rects, const Vector<GapRects>& gapRects, bool isTextOnly)
Modified: branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h (172552 => 172553)
--- branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h 2014-08-14 03:19:02 UTC (rev 172552)
+++ branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h 2014-08-14 03:38:54 UTC (rev 172553)
@@ -169,7 +169,7 @@
virtual bool supportsGlobalSelection() override;
#if ENABLE(TELEPHONE_NUMBER_DETECTION) || ENABLE(SERVICE_CONTROLS)
- virtual void selectedTelephoneNumberRangesChanged(const Vector<RefPtr<WebCore::Range>>&) override;
+ virtual void selectedTelephoneNumberRangesChanged() override;
virtual void selectionRectsDidChange(const Vector<WebCore::LayoutRect>&, const Vector<WebCore::GapRects>&, bool isTextOnly) override;
#endif
Modified: branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h (172552 => 172553)
--- branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h 2014-08-14 03:19:02 UTC (rev 172552)
+++ branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h 2014-08-14 03:38:54 UTC (rev 172553)
@@ -29,6 +29,7 @@
#if ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
#include "PageOverlay.h"
+#include "WebFrame.h"
#include <WebCore/Range.h>
#include <WebCore/Timer.h>
#include <wtf/RefCounted.h>
@@ -50,7 +51,7 @@
ServicesOverlayController(WebPage&);
~ServicesOverlayController();
- void selectedTelephoneNumberRangesChanged(const Vector<RefPtr<WebCore::Range>>&);
+ void selectedTelephoneNumberRangesChanged();
void selectionRectsDidChange(const Vector<WebCore::LayoutRect>&, const Vector<WebCore::GapRects>&, bool isTextOnly);
private:
@@ -112,13 +113,14 @@
static bool highlightsAreEquivalent(const Highlight* a, const Highlight* b);
+ Vector<RefPtr<WebCore::Range>> telephoneNumberRangesForFocusedFrame();
+
WebPage& m_webPage;
PageOverlay* m_servicesOverlay;
RefPtr<Highlight> m_activeHighlight;
HashSet<RefPtr<Highlight>> m_potentialHighlights;
- Vector<RefPtr<WebCore::Range>> m_currentTelephoneNumberRanges;
Vector<WebCore::LayoutRect> m_currentSelectionRects;
bool m_isTextOnly;
Modified: branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm (172552 => 172553)
--- branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm 2014-08-14 03:19:02 UTC (rev 172552)
+++ branches/safari-600.1-branch/Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm 2014-08-14 03:38:54 UTC (rev 172553)
@@ -33,6 +33,7 @@
#import "WebProcess.h"
#import <WebCore/Document.h>
#import <WebCore/FloatQuad.h>
+#import <WebCore/FocusController.h>
#import <WebCore/FrameView.h>
#import <WebCore/GapRects.h>
#import <WebCore/GraphicsContext.h>
@@ -224,12 +225,10 @@
#endif
}
-void ServicesOverlayController::selectedTelephoneNumberRangesChanged(const Vector<RefPtr<Range>>& ranges)
+void ServicesOverlayController::selectedTelephoneNumberRangesChanged()
{
#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED > 1090
- LOG(Services, "ServicesOverlayController - Telephone number ranges changed - Had %lu, now have %lu\n", m_currentTelephoneNumberRanges.size(), ranges.size());
- m_currentTelephoneNumberRanges = ranges;
-
+ LOG(Services, "ServicesOverlayController - Telephone number ranges changed\n");
buildPhoneNumberHighlights();
#else
UNUSED_PARAM(ranges);
@@ -323,25 +322,30 @@
{
removeAllPotentialHighlightsOfType(Highlight::Type::TelephoneNumber);
- for (unsigned i = 0; i < m_currentTelephoneNumberRanges.size(); ++i) {
- // FIXME: This will choke if the range wraps around the edge of the view.
- // What should we do in that case?
- IntRect rect = textQuadsToBoundingRectForRange(*m_currentTelephoneNumberRanges[i]);
+ Frame* mainFrame = m_webPage.mainFrame();
+ FrameView& mainFrameView = *mainFrame->view();
- // Convert to the main document's coordinate space.
- // FIXME: It's a little crazy to call contentsToWindow and then windowToContents in order to get the right coordinate space.
- // We should consider adding conversion functions to ScrollView for contentsToDocument(). Right now, contentsToRootView() is
- // not equivalent to what we need when you have a topContentInset or a header banner.
- FrameView* viewForRange = m_currentTelephoneNumberRanges[i]->ownerDocument().view();
- if (!viewForRange)
- continue;
- FrameView& mainFrameView = *m_webPage.corePage()->mainFrame().view();
- rect.setLocation(mainFrameView.windowToContents(viewForRange->contentsToWindow(rect.location())));
+ for (Frame* frame = mainFrame; frame; frame = frame->tree().traverseNext()) {
+ auto& ranges = frame->editor().detectedTelephoneNumberRanges();
+ for (auto& range : ranges) {
+ // FIXME: This will choke if the range wraps around the edge of the view.
+ // What should we do in that case?
+ IntRect rect = textQuadsToBoundingRectForRange(*range);
- CGRect cgRect = rect;
- RetainPtr<DDHighlightRef> ddHighlight = adoptCF(DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection(nullptr, &cgRect, 1, mainFrameView.visibleContentRect(), DDHighlightOutlineWithArrow, YES, NSWritingDirectionNatural, NO, YES));
+ // Convert to the main document's coordinate space.
+ // FIXME: It's a little crazy to call contentsToWindow and then windowToContents in order to get the right coordinate space.
+ // We should consider adding conversion functions to ScrollView for contentsToDocument(). Right now, contentsToRootView() is
+ // not equivalent to what we need when you have a topContentInset or a header banner.
+ FrameView* viewForRange = range->ownerDocument().view();
+ if (!viewForRange)
+ continue;
+ rect.setLocation(mainFrameView.windowToContents(viewForRange->contentsToWindow(rect.location())));
- m_potentialHighlights.add(Highlight::createForTelephoneNumber(ddHighlight, m_currentTelephoneNumberRanges[i]));
+ CGRect cgRect = rect;
+ RetainPtr<DDHighlightRef> ddHighlight = adoptCF(DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection(nullptr, &cgRect, 1, mainFrameView.visibleContentRect(), DDHighlightOutlineWithArrow, YES, NSWritingDirectionNatural, NO, YES));
+
+ m_potentialHighlights.add(Highlight::createForTelephoneNumber(ddHighlight, range));
+ }
}
didRebuildPotentialHighlights();
@@ -380,7 +384,7 @@
return;
}
- if (m_currentTelephoneNumberRanges.isEmpty() && !hasRelevantSelectionServices())
+ if (telephoneNumberRangesForFocusedFrame().isEmpty() && !hasRelevantSelectionServices())
return;
createOverlayIfNeeded();
@@ -399,6 +403,15 @@
m_servicesOverlay->setNeedsDisplay();
}
+Vector<RefPtr<Range>> ServicesOverlayController::telephoneNumberRangesForFocusedFrame()
+{
+ Page* page = m_webPage.corePage();
+ if (!page)
+ return Vector<RefPtr<Range>>();
+
+ return page->focusController().focusedOrMainFrame().editor().detectedTelephoneNumberRanges();
+}
+
bool ServicesOverlayController::highlightsAreEquivalent(const Highlight* a, const Highlight* b)
{
if (a == b)
@@ -427,7 +440,7 @@
continue;
// If this highlight has no compatible services, it can't be active, unless we have telephone number highlights to show in the combined menu.
- if (m_currentTelephoneNumberRanges.isEmpty() && !hasRelevantSelectionServices())
+ if (telephoneNumberRangesForFocusedFrame().isEmpty() && !hasRelevantSelectionServices())
continue;
}
@@ -505,9 +518,10 @@
IntPoint windowPoint = frameView->contentsToWindow(clickPoint);
if (highlight.type() == Highlight::Type::Selection) {
+ auto telephoneNumberRanges = telephoneNumberRangesForFocusedFrame();
Vector<String> selectedTelephoneNumbers;
- selectedTelephoneNumbers.reserveCapacity(m_currentTelephoneNumberRanges.size());
- for (auto& range : m_currentTelephoneNumberRanges)
+ selectedTelephoneNumbers.reserveCapacity(telephoneNumberRanges.size());
+ for (auto& range : telephoneNumberRanges)
selectedTelephoneNumbers.append(range->text());
m_webPage.handleSelectionServiceClick(m_webPage.corePage()->mainFrame().selection(), selectedTelephoneNumbers, windowPoint);
