Summary: | During a copy, position:fixed gets converted to position:absolute even if only part of the document is selected | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||
Component: | HTML Editing | Assignee: | Myles C. Maxfield <mmaxfield> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | dino, jonlee, rniwa, simon.fraser, thorton, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Myles C. Maxfield
2014-02-12 14:05:30 PST
Created attachment 224005 [details]
Patch
Comment on attachment 224005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224005&action=review > Source/WebCore/editing/markup.cpp:627 > - if (accumulator.needRelativeStyleWrapper() && body && fullySelectedRoot == body) { > + if (accumulator.needRelativeStyleWrapper() && needsPositionStyleConversion) { I don't think we should do this. This would result in another round of Apple-style-span nightmare. Comment on attachment 224005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224005&action=review > Source/WebCore/editing/markup.cpp:328 > + if (m_needsPositionStyleConversion && element.document().settings() && element.document().settings()->shouldConvertPositionStyleOnCopy()) > m_needRelativeStyleWrapper |= newInlineStyle->convertFixedAndStickyPosition(); This is actually broken because convertFixedAndStickyPosition doesn't adjust coordinates with respect to the wrapping div's. r-. Comment on attachment 224005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224005&action=review >> Source/WebCore/editing/markup.cpp:328 >> m_needRelativeStyleWrapper |= newInlineStyle->convertFixedAndStickyPosition(); > > This is actually broken because convertFixedAndStickyPosition doesn't adjust coordinates with respect to the wrapping div's. r-. Oh oops, I completely misunderstood this patch :( I think we should check settings in createMarkupInternal though. > Source/WebCore/editing/markup.cpp:570 > + bool needsPositionStyleConversion = body && fullySelectedRoot == body; We don't need this separate boolean because fullySelectedRoot can only ever set to body. >> Source/WebCore/editing/markup.cpp:627 >> + if (accumulator.needRelativeStyleWrapper() && needsPositionStyleConversion) { > > I don't think we should do this. This would result in another round of Apple-style-span nightmare. Ignore this comment. Comment on attachment 224005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224005&action=review >> Source/WebCore/editing/markup.cpp:570 >> + bool needsPositionStyleConversion = body && fullySelectedRoot == body; > > We don't need this separate boolean because fullySelectedRoot can only ever set to body. I'm going to keep the separate boolean because it gets read twice, and it is conceptually different than the comparison that it gets set to Comment on attachment 224005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224005&action=review >>> Source/WebCore/editing/markup.cpp:328 >>> m_needRelativeStyleWrapper |= newInlineStyle->convertFixedAndStickyPosition(); >> >> This is actually broken because convertFixedAndStickyPosition doesn't adjust coordinates with respect to the wrapping div's. r-. > > Oh oops, I completely misunderstood this patch :( > > I think we should check settings in createMarkupInternal though. Done. |