[Webkit-unassigned] [Bug 118272] Take document height into account when determining when it is considered visually non-empy
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jul 2 12:18:47 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=118272
Darin Adler <darin at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #205852|review? |review+
Flag| |
--- Comment #2 from Darin Adler <darin at apple.com> 2013-07-02 12:20:46 PST ---
(From update of attachment 205852)
View in context: https://bugs.webkit.org/attachment.cgi?id=205852&action=review
> Source/WebCore/ChangeLog:8
> + The current visually non-empy mechanism takes into account only the amount of contents in renderers.
Typo: "empy".
> Source/WebCore/page/FrameView.cpp:3721
> +bool FrameView::determingIsVisuallyNonEmpty() const
Typo here: “determing”. Confusing function name, “determine is visually non empty”. Maybe “isNowVisuallyNonEmpty” or “isVisuallyNonEmptyUncached”. Neither of those is good, but I don’t like “determine is” better. Maybe “qualifiesAsVisuallyNonEmpty” is a good one.
> Source/WebCore/page/FrameView.cpp:3742
> + // No content yet.
> + Element* documentElement = m_frame->document()->documentElement();
> + if (!documentElement || !documentElement->renderer())
> + return false;
> + // Ensure that we always get marked visually non-empty eventually.
> + if (!m_frame->document()->parsing() && m_frame->loader()->stateMachine()->committedFirstRealDocumentLoad())
> + return true;
> + // Require the document to grow a bit.
> + static const int documentHeightThreshold = 200;
> + if (documentElement->renderBox()->layoutOverflowRect().pixelSnappedHeight() < documentHeightThreshold)
> + return false;
> + // The first few hundred characters rarely contain the interesting content of the page.
> + static const unsigned visualCharacterThreshold = 200;
> + if (m_visuallyNonEmptyCharacterCount > visualCharacterThreshold)
> + return true;
> + // Use a threshold value to prevent very small amounts of visible content from triggering didFirstVisuallyNonEmptyLayout
> + static const unsigned visualPixelThreshold = 32 * 32;
> + if (m_visuallyNonEmptyPixelCount > visualPixelThreshold)
> + return true;
> + return false;
I think the way this alternates false with true early returns makes it tricky to read the logic. Maybe an extra blank line to paragraph the sections could make it clearer?
> Source/WebCore/rendering/RenderHTMLCanvas.cpp:50
> {
> - view()->frameView()->setIsVisuallyNonEmpty();
> + // Actual size is not known yet, report the default intrinsic size.
> + view()->frameView()->incrementVisuallyNonEmptyPixelCount(roundedIntSize(intrinsicSize()));
> +
> }
Excess blank line being added here.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list