1808886 - [Yahoo mail] Copying formatted text on mail.yahoo.com does not respect text size changes

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Attachment: Screen Recording 2023-01-06 at 11.39.33.mov

Notes

• This does not reproduce in Chrome. Please see the attachment for more details.

Found in

• Nightly 110.0a1

Affected versions

• Nightly 110.0a1
• Firefox 109.0b9
• Firefox 108.0.2

Tested platforms

• macOS 12
• Ubuntu 22
• Windows 10

Affected platforms

• macOS 12
• Ubuntu 22
• Windows 10

Unaffected platforms

• N/A;

Steps to reproduce

1. Launch Firefox and go to https://mail.yahoo.com.
2. Log in with a valid account.
3. Click on the “Compose” button to open a new e-mail window.
4. Open a new tab and go to https://docs.google.com/.
5. Log in with a valid account.
6. Open a new Blank Document and write one or two phrases and apply formatting( text size, highlights, different fonts).
7. Return to https://mail.yahoo.com tab and paste the formatted text into the e-mail window.
8. Select the newly-pasted phrases and change the size of text.

Expected result

• Text size changes accordingly to reflect the user’s selection.

Actual result

• Text size does not change.

Regression range

• Will look for one as soon as possible.

Comment 1

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

I tried searching for a regression range but it seems that I can reproduce the issue up until Nightly 71.0a1(build ID: 20190922213341).
I think it's safe to assume that this is not a regression.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

I've not investigated this, but I guess that <font size> is inserted inside or outside of <span style="font-size:...">. Currently, our style editor checks HTML elements representing a style in the CSS mode, but does not do the opposite thing, i.e., checking
CSS style of elements, in the HTML mode. Perhaps, we need to make the style editor removes CSS style in the HTML mode.

• https://searchfox.org/mozilla-central/rev/893a8f062ec6144c84403fbfb0a57234418b89cf/editor/libeditor/HTMLEditHelpers.cpp#146

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

Oh, no, CSSEditUtils::GetCSSDeclarations does not handle <font size> style...
https://searchfox.org/mozilla-central/rev/b19830c6b22f73758862d34d2c64f57735890e90/editor/libeditor/CSSEditUtils.cpp#815-822

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

Hmm, "Don't use CSS for <font size>, we don't support it usefully (bug 780035)"
https://searchfox.org/mozilla-central/rev/b19830c6b22f73758862d34d2c64f57735890e90/editor/libeditor/HTMLEditSubActionHandler.cpp#9182

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

Attachment: Bug 1808886 - Make the style editor treat `<font>` related styles as exclusive styles r=m_kato!

Currently, Gecko does not support updating font-size style even in the CSS
mode. Therefore, CSSEditUtils claims that <font size="..."> style is not
CSS editable. Supporting it requires a lot of new code but currently we don't
have any reports about this poor support. Therefore, we should only try to
make the style editor remove font-size properties around selection at setting
<font size="..."> style for now.

Additionally, Blink and WebKit does not allow to nest multiple <font> elements
and <span> elements which specify font-size, font-family or color. When
setting one of the styles, they split ancestors which are related to the style
at range boundaries. Then, unwrap the unnecessary elements. This makes the
DOM tree simpler, thus, the later handling will be simpler. Therefore, it's
worthwhile to follow them.

Unfortunately, there are some logic differences when handling styles in elements
having id attribute. Therefore, there are some new test failures, but I guess
that it does not cause any problems in web apps in the wild because it's hard to
manage editable identified elements in cross browsers and I have no idea of
motivations to do it.

Depends on D166617

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e891dce2c845
Make the style editor treat `<font>` related styles as exclusive styles r=m_kato

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/38077 for changes under testing/web-platform/tests

Comment 8

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/e891dce2c845

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:masayuki, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox110 to wontfix.

For more information, please visit auto_nag documentation.

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

Risky and not a new regression. So wontfix for beta is reasonable.