- Revision
- 202117
- Author
- [email protected]
- Date
- 2016-06-15 18:14:02 -0700 (Wed, 15 Jun 2016)
Log Message
<attachment> elements jump around a lot around when subtitle text changes slightly
https://bugs.webkit.org/show_bug.cgi?id=158818
<rdar://problem/24450270>
Reviewed by Simon Fraser.
Test: fast/attachment/attachment-subtitle-resize.html
* rendering/RenderAttachment.cpp:
(WebCore::RenderAttachment::layout):
* rendering/RenderAttachment.h:
* rendering/RenderThemeMac.mm:
(WebCore::AttachmentLayout::AttachmentLayout):
(WebCore::RenderThemeMac::paintAttachment):
In order to avoid changes to the centered subtitle text causing the whole
attachment to bounce around a lot, make it so that attachment width can only
increase, never decrease, and round the subtitle's width up to the nearest
increment of 10px when determining its affect on the whole element's width.
Also, center the attachment in its element, instead of left-aligning it,
so that the extra width we may have is evenly distributed between the two sides.
* fast/attachment/attachment-subtitle-resize-expected.txt: Added.
* fast/attachment/attachment-subtitle-resize.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (202116 => 202117)
--- trunk/LayoutTests/ChangeLog 2016-06-15 23:44:57 UTC (rev 202116)
+++ trunk/LayoutTests/ChangeLog 2016-06-16 01:14:02 UTC (rev 202117)
@@ -1,3 +1,14 @@
+2016-06-15 Tim Horton <[email protected]>
+
+ <attachment> elements jump around a lot around when subtitle text changes slightly
+ https://bugs.webkit.org/show_bug.cgi?id=158818
+ <rdar://problem/24450270>
+
+ Reviewed by Simon Fraser.
+
+ * fast/attachment/attachment-subtitle-resize-expected.txt: Added.
+ * fast/attachment/attachment-subtitle-resize.html: Added.
+
2016-06-13 Simon Fraser <[email protected]>
[iOS WK2] Make it possible to test the Next/Previous buttons in the keyboard accessory bar
Modified: trunk/LayoutTests/TestExpectations (202116 => 202117)
--- trunk/LayoutTests/TestExpectations 2016-06-15 23:44:57 UTC (rev 202116)
+++ trunk/LayoutTests/TestExpectations 2016-06-16 01:14:02 UTC (rev 202117)
@@ -54,6 +54,9 @@
fast/harness/ui-side-scripts.html [ Skip ]
fast/harness/concurrent-ui-side-scripts.html [ Skip ]
+# This test only makes sense on Mac
+fast/attachment/attachment-subtitle-resize.html
+
#//////////////////////////////////////////////////////////////////////////////////////////
# End platform-specific tests.
#//////////////////////////////////////////////////////////////////////////////////////////
Added: trunk/LayoutTests/fast/attachment/attachment-subtitle-resize-expected.txt (0 => 202117)
--- trunk/LayoutTests/fast/attachment/attachment-subtitle-resize-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/attachment/attachment-subtitle-resize-expected.txt 2016-06-16 01:14:02 UTC (rev 202117)
@@ -0,0 +1,13 @@
+This tests that attachments which resize will only grow in width, never shrink.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS wideAttachmentWidth > skinnyAttachmentWidth
+PASS Attachment starts out skinny.
+PASS Attachment resized from skinny to wide becomes wide.
+PASS Attachment resized from wide to skinny remains wide.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/attachment/attachment-subtitle-resize.html (0 => 202117)
--- trunk/LayoutTests/fast/attachment/attachment-subtitle-resize.html (rev 0)
+++ trunk/LayoutTests/fast/attachment/attachment-subtitle-resize.html 2016-06-16 01:14:02 UTC (rev 202117)
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<script src=""
+<script>
+ if (window.internals)
+ window.internals.settings.setAttachmentElementEnabled(true)
+</script>
+<body>
+<attachment id="wideAttachment" subtitle="VERY LONG SUBTITLE THAT WILL MAKE THE ATTACHMENT VERY WIDE"></attachment>
+<attachment id="skinnyAttachment" subtitle="skinny but wider than icon"></attachment>
+<attachment id="resizedAttachment" subtitle="skinny but wider than icon"></attachment>
+<script>
+description("This tests that attachments which resize will only grow in width, never shrink.");
+
+var wideAttachmentEl = document.getElementById("wideAttachment");
+var skinnyAttachmentEl = document.getElementById("skinnyAttachment");
+var resizedAttachmentEl = document.getElementById("resizedAttachment");
+
+var wideAttachmentWidth = wideAttachmentEl.offsetWidth;
+var skinnyAttachmentWidth = skinnyAttachmentEl.offsetWidth;
+
+function assert(value, description)
+{
+ if (value)
+ testPassed(description);
+ else
+ testFailed(description);
+}
+
+assert(wideAttachmentWidth > skinnyAttachmentWidth, "wideAttachmentWidth > skinnyAttachmentWidth");
+
+assert(resizedAttachmentEl.offsetWidth == skinnyAttachmentWidth, "Attachment starts out skinny.");
+
+resizedAttachmentEl.setAttribute("subtitle", "VERY LONG SUBTITLE THAT WILL MAKE THE ATTACHMENT VERY WIDE");
+assert(resizedAttachmentEl.offsetWidth == wideAttachmentWidth, "Attachment resized from skinny to wide becomes wide.");
+
+resizedAttachmentEl.setAttribute("subtitle", "skinny but wider than icon");
+assert(resizedAttachmentEl.offsetWidth == wideAttachmentWidth, "Attachment resized from wide to skinny remains wide.");
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/platform/mac/TestExpectations (202116 => 202117)
--- trunk/LayoutTests/platform/mac/TestExpectations 2016-06-15 23:44:57 UTC (rev 202116)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2016-06-16 01:14:02 UTC (rev 202117)
@@ -20,6 +20,8 @@
fast/text-autosizing/ios [ Pass ]
fast/text-autosizing/mac [ Pass ]
+fast/attachment/attachment-subtitle-resize.html [ Pass ]
+
#//////////////////////////////////////////////////////////////////////////////////////////
# End platform-specific directories.
#//////////////////////////////////////////////////////////////////////////////////////////
Modified: trunk/Source/WebCore/ChangeLog (202116 => 202117)
--- trunk/Source/WebCore/ChangeLog 2016-06-15 23:44:57 UTC (rev 202116)
+++ trunk/Source/WebCore/ChangeLog 2016-06-16 01:14:02 UTC (rev 202117)
@@ -1,3 +1,26 @@
+2016-06-15 Tim Horton <[email protected]>
+
+ <attachment> elements jump around a lot around when subtitle text changes slightly
+ https://bugs.webkit.org/show_bug.cgi?id=158818
+ <rdar://problem/24450270>
+
+ Reviewed by Simon Fraser.
+
+ Test: fast/attachment/attachment-subtitle-resize.html
+
+ * rendering/RenderAttachment.cpp:
+ (WebCore::RenderAttachment::layout):
+ * rendering/RenderAttachment.h:
+ * rendering/RenderThemeMac.mm:
+ (WebCore::AttachmentLayout::AttachmentLayout):
+ (WebCore::RenderThemeMac::paintAttachment):
+ In order to avoid changes to the centered subtitle text causing the whole
+ attachment to bounce around a lot, make it so that attachment width can only
+ increase, never decrease, and round the subtitle's width up to the nearest
+ increment of 10px when determining its affect on the whole element's width.
+ Also, center the attachment in its element, instead of left-aligning it,
+ so that the extra width we may have is evenly distributed between the two sides.
+
2016-06-15 Ryan Haddad <[email protected]>
Reset bindings test results after r202105
Modified: trunk/Source/WebCore/rendering/RenderAttachment.cpp (202116 => 202117)
--- trunk/Source/WebCore/rendering/RenderAttachment.cpp 2016-06-15 23:44:57 UTC (rev 202116)
+++ trunk/Source/WebCore/rendering/RenderAttachment.cpp 2016-06-16 01:14:02 UTC (rev 202117)
@@ -53,7 +53,10 @@
void RenderAttachment::layout()
{
- setIntrinsicSize(document().page()->theme().attachmentIntrinsicSize(*this));
+ LayoutSize newIntrinsicSize = document().page()->theme().attachmentIntrinsicSize(*this);
+ m_minimumIntrinsicWidth = std::max(m_minimumIntrinsicWidth, newIntrinsicSize.width());
+ newIntrinsicSize.setWidth(m_minimumIntrinsicWidth);
+ setIntrinsicSize(newIntrinsicSize);
RenderReplaced::layout();
}
Modified: trunk/Source/WebCore/rendering/RenderAttachment.h (202116 => 202117)
--- trunk/Source/WebCore/rendering/RenderAttachment.h 2016-06-15 23:44:57 UTC (rev 202116)
+++ trunk/Source/WebCore/rendering/RenderAttachment.h 2016-06-16 01:14:02 UTC (rev 202117)
@@ -50,6 +50,8 @@
void layout() override;
int baselinePosition(FontBaseline, bool, LineDirectionMode, LinePositionMode) const override;
+
+ LayoutUnit m_minimumIntrinsicWidth;
};
inline RenderAttachment* HTMLAttachmentElement::renderer() const
Modified: trunk/Source/WebCore/rendering/RenderThemeMac.mm (202116 => 202117)
--- trunk/Source/WebCore/rendering/RenderThemeMac.mm 2016-06-15 23:44:57 UTC (rev 202116)
+++ trunk/Source/WebCore/rendering/RenderThemeMac.mm 2016-06-16 01:14:02 UTC (rev 202117)
@@ -73,6 +73,7 @@
#import "UserAgentScripts.h"
#import "UserAgentStyleSheets.h"
#import "WebCoreSystemInterface.h"
+#import <wtf/MathExtras.h>
#import <wtf/RetainPtr.h>
#import <wtf/RetainPtr.h>
#import <wtf/StdLibExtras.h>
@@ -2188,6 +2189,7 @@
static Color attachmentTitleInactiveTextColor() { return Color(100, 100, 100, 255); }
const CGFloat attachmentSubtitleFontSize = 10;
+const int attachmentSubtitleWidthIncrement = 10;
static Color attachmentSubtitleTextColor() { return Color(82, 145, 214, 255); }
const CGFloat attachmentProgressBarWidth = 30;
@@ -2381,7 +2383,11 @@
attachmentRect = iconBackgroundRect;
for (const auto& line : lines)
attachmentRect.unite(line.backgroundRect);
- attachmentRect.unite(subtitleTextRect);
+ if (!subtitleTextRect.isEmpty()) {
+ FloatRect roundedSubtitleTextRect = subtitleTextRect;
+ roundedSubtitleTextRect.inflateX(attachmentSubtitleWidthIncrement - clampToInteger(ceilf(subtitleTextRect.width())) % attachmentSubtitleWidthIncrement);
+ attachmentRect.unite(roundedSubtitleTextRect);
+ }
attachmentRect.inflate(attachmentMargin);
attachmentRect = encloseRectToDevicePixels(attachmentRect, attachment.document().deviceScaleFactor());
}
@@ -2594,7 +2600,7 @@
GraphicsContextStateSaver saver(context);
context.translate(toFloatSize(paintRect.location()));
- context.translate(FloatSize((layout.attachmentRect.width() - attachmentIconBackgroundSize) / 2, 0));
+ context.translate(floorSizeToDevicePixels(LayoutSize((paintRect.width() - attachmentIconBackgroundSize) / 2, 0), renderer.document().deviceScaleFactor()));
bool useSelectedStyle = attachment.selectionState() != RenderObject::SelectionNone;
bool usePlaceholder = validProgress && !progress;
