Changeset 265059 in webkit


Ignore:
Timestamp:
Jul 29, 2020 2:50:47 PM (4 years ago)
Author:
Wenson Hsieh
Message:

[macOS] Inspector bar in Mail compose shows incorrect text alignment style for ranged selections
​https://bugs.webkit.org/show_bug.cgi?id=214930
<rdar://problem/66185224>

Reviewed by Tim Horton.

Source/WebCore:

When a range of text is selected, AppKit consults -attributedSubstringForProposedRange:completionHandler: (or
just -attributedSubstringFromRange: in WebKitLegacy) to update the inspector bar with new style information.
Among this information is the paragraph style (an NSParagraphStyle corresponding to the attribute key
NSParagraphStyleAttributeName), which AppKit uses to select either the left, center or right text alignment
segmented control.

However, in both WebKitLegacy and modern WebKit, we don't include this information at all in HTMLConverter,
so AppKit just defaults to using -[NSParagraphStyle defaultParagraphStyle], with a default text alignment of
NSTextAlignmentNatural.

To fix this, include a new paragraph style object in the case where the text alignment is not equal to natural
(in other words, either a direction has been explicitly specified, or the text alignment style is not equal to
"start"). This paragraph style matches the default paragraph style, except for the alignment property which
is set to the corresponding NSTextAlignment value.

Tests: AttributedSubstringForProposedRange.TextAlignmentParagraphStyles

FontAttributes.FontAttributesAfterChangingSelection

  • editing/Editor.cpp:

(WebCore::Editor::fontAttributesAtSelectionStart const):

Also fixes an existing bug in Editor::fontAttributesAtSelectionStart, where NSTextAlignmentNatural is
currently always used for text-align: start;, even if the direction is specified as RTL. Instead, change this
so that we only fall back to "natural" if no direction is specified; otherwise, go with the explicit "left" or
"right" values.

  • editing/cocoa/HTMLConverter.mm:

(WebCore::editingAttributedString):

Tools:

Add a new API test and adjust an existing test to verify the codechange.

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/TestWebKitAPI/mac/AppKitSPI.h:
  • TestWebKitAPI/Tests/WebKitCocoa/FontAttributes.mm:

(TestWebKitAPI::TEST):

  • TestWebKitAPI/Tests/WebKitCocoa/rich-text-attributes.html:
  • TestWebKitAPI/Tests/mac/AttributedSubstringForProposedRange.mm: Added.

(TEST):

Location:
trunk
Files:
1 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r265056 r265059  
     12020-07-29  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [macOS] Inspector bar in Mail compose shows incorrect text alignment style for ranged selections
     4        https://bugs.webkit.org/show_bug.cgi?id=214930
     5        <rdar://problem/66185224>
     6
     7        Reviewed by Tim Horton.
     8
     9        When a range of text is selected, AppKit consults `-attributedSubstringForProposedRange:completionHandler:` (or
     10        just `-attributedSubstringFromRange:` in WebKitLegacy) to update the inspector bar with new style information.
     11        Among this information is the paragraph style (an `NSParagraphStyle` corresponding to the attribute key
     12        `NSParagraphStyleAttributeName`), which AppKit uses to select either the left, center or right text alignment
     13        segmented control.
     14
     15        However, in both WebKitLegacy and modern WebKit, we don't include this information at all in HTMLConverter,
     16        so AppKit just defaults to using `-[NSParagraphStyle defaultParagraphStyle]`, with a default text alignment of
     17        `NSTextAlignmentNatural`.
     18
     19        To fix this, include a new paragraph style object in the case where the text alignment is not equal to natural
     20        (in other words, either a direction has been explicitly specified, or the text alignment style is not equal to
     21        "start"). This paragraph style matches the default paragraph style, except for the `alignment` property which
     22        is set to the corresponding `NSTextAlignment` value.
     23
     24        Tests:  AttributedSubstringForProposedRange.TextAlignmentParagraphStyles
     25                FontAttributes.FontAttributesAfterChangingSelection
     26
     27        * editing/Editor.cpp:
     28        (WebCore::Editor::fontAttributesAtSelectionStart const):
     29
     30        Also fixes an existing bug in `Editor::fontAttributesAtSelectionStart`, where `NSTextAlignmentNatural` is
     31        currently always used for `text-align: start;`, even if the direction is specified as RTL. Instead, change this
     32        so that we only fall back to "natural" if no direction is specified; otherwise, go with the explicit "left" or
     33        "right" values.
     34
     35        * editing/cocoa/HTMLConverter.mm:
     36        (WebCore::editingAttributedString):
     37
    1382020-07-29  Jer Noble  <jer.noble@apple.com>
    239

  • trunk/Source/WebCore/editing/Editor.cpp

    r265044 r265059  
    40354035        break;
    40364036    case TextAlignMode::Start:
    4037         attributes.horizontalAlignment = FontAttributes::HorizontalAlignment::Natural;
     4037        if (style->hasExplicitlySetDirection())
     4038            attributes.horizontalAlignment = style->isLeftToRightDirection() ? FontAttributes::HorizontalAlignment::Left : FontAttributes::HorizontalAlignment::Right;
     4039        else
     4040            attributes.horizontalAlignment = FontAttributes::HorizontalAlignment::Natural;
    40384041        break;
    40394042    case TextAlignMode::End:

  • trunk/Source/WebCore/editing/cocoa/HTMLConverter.mm

    r262683 r265059  
    24102410            [attrs setObject:[fontManager convertFont:WebDefaultFont() toSize:style.fontCascade().primaryFont().platformData().size()] forKey:NSFontAttributeName];
    24112411
     2412        auto textAlignment = NSTextAlignmentNatural;
     2413        switch (style.textAlign()) {
     2414        case TextAlignMode::Right:
     2415        case TextAlignMode::WebKitRight:
     2416            textAlignment = NSTextAlignmentRight;
     2417            break;
     2418        case TextAlignMode::Left:
     2419        case TextAlignMode::WebKitLeft:
     2420            textAlignment = NSTextAlignmentLeft;
     2421            break;
     2422        case TextAlignMode::Center:
     2423        case TextAlignMode::WebKitCenter:
     2424            textAlignment = NSTextAlignmentCenter;
     2425            break;
     2426        case TextAlignMode::Justify:
     2427            textAlignment = NSTextAlignmentJustified;
     2428            break;
     2429        case TextAlignMode::Start:
     2430            if (style.hasExplicitlySetDirection())
     2431                textAlignment = style.isLeftToRightDirection() ? NSTextAlignmentLeft : NSTextAlignmentRight;
     2432            break;
     2433        case TextAlignMode::End:
     2434            textAlignment = style.isLeftToRightDirection() ? NSTextAlignmentRight : NSTextAlignmentLeft;
     2435            break;
     2436        default:
     2437            ASSERT_NOT_REACHED();
     2438            break;
     2439        }
     2440
     2441        if (textAlignment != NSTextAlignmentNatural) {
     2442            auto paragraphStyle = adoptNS(NSParagraphStyle.defaultParagraphStyle.mutableCopy);
     2443            [paragraphStyle setAlignment:textAlignment];
     2444            [attrs setObject:paragraphStyle.get() forKey:NSParagraphStyleAttributeName];
     2445        }
     2446
    24122447        Color foregroundColor = style.visitedDependentColorWithColorFilter(CSSPropertyColor);
    24132448        if (foregroundColor.isVisible())

  • trunk/Tools/ChangeLog

    r265058 r265059  
     12020-07-29  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [macOS] Inspector bar in Mail compose shows incorrect text alignment style for ranged selections
     4        https://bugs.webkit.org/show_bug.cgi?id=214930
     5        <rdar://problem/66185224>
     6
     7        Reviewed by Tim Horton.
     8
     9        Add a new API test and adjust an existing test to verify the codechange.
     10
     11        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     12        * TestWebKitAPI/Tests/TestWebKitAPI/mac/AppKitSPI.h:
     13        * TestWebKitAPI/Tests/WebKitCocoa/FontAttributes.mm:
     14        (TestWebKitAPI::TEST):
     15        * TestWebKitAPI/Tests/WebKitCocoa/rich-text-attributes.html:
     16        * TestWebKitAPI/Tests/mac/AttributedSubstringForProposedRange.mm: Added.
     17        (TEST):
     18
    1192020-07-29  Jonathan Bedard  <jbedard@apple.com>
    220

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r264937 r265059  
    11671167                F4F5BB5221667BAA002D06B9 /* TestFontOptions.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4F5BB5121667BAA002D06B9 /* TestFontOptions.mm */; };
    11681168                F4FA282A24CD012700618A46 /* FormValidation.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4FA282924CD012700618A46 /* FormValidation.mm */; };
     1169                F4FA2A4F24D1F05700618A46 /* AttributedSubstringForProposedRange.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4FA2A4E24D1F05700618A46 /* AttributedSubstringForProposedRange.mm */; };
    11691170                F4FA91811E61849B007B8C1D /* WKWebViewMacEditingTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4FA917F1E61849B007B8C1D /* WKWebViewMacEditingTests.mm */; };
    11701171                F4FA91831E61857B007B8C1D /* double-click-does-not-select-trailing-space.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4FA91821E618566007B8C1D /* double-click-does-not-select-trailing-space.html */; };
    … …  
    28802881                F4F5BB5121667BAA002D06B9 /* TestFontOptions.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = TestFontOptions.mm; sourceTree = "<group>"; };
    28812882                F4FA282924CD012700618A46 /* FormValidation.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FormValidation.mm; sourceTree = "<group>"; };
     2883                F4FA2A4E24D1F05700618A46 /* AttributedSubstringForProposedRange.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = AttributedSubstringForProposedRange.mm; sourceTree = "<group>"; };
    28822884                F4FA917F1E61849B007B8C1D /* WKWebViewMacEditingTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebViewMacEditingTests.mm; sourceTree = "<group>"; };
    28832885                F4FA91821E618566007B8C1D /* double-click-does-not-select-trailing-space.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; name = "double-click-does-not-select-trailing-space.html"; path = "Tests/WebKitCocoa/double-click-does-not-select-trailing-space.html"; sourceTree = SOURCE_ROOT; };
    … …  
    43384340                                55F9D2E42205031800A9AB38 /* AdditionalSupportedImageTypes.mm */,
    43394341                                B55F119F1516834F00915916 /* AttributedString.mm */,
     4342                                F4FA2A4E24D1F05700618A46 /* AttributedSubstringForProposedRange.mm */,
    43404343                                00CD9F6215BE312C002DA2CE /* BackForwardList.mm */,
    43414344                                1C7FEB1F207C0F2D00D23278 /* BackgroundColor.mm */,
    … …  
    49374940                                834138C7203261CA00F26960 /* AsyncPolicyForNavigationResponse.mm in Sources */,
    49384941                                7CCE7EB41A411A7E00447C4C /* AttributedString.mm in Sources */,
     4942                                F4FA2A4F24D1F05700618A46 /* AttributedSubstringForProposedRange.mm in Sources */,
    49394943                                3760C4F1211249AF00233ACC /* AttrStyle.mm in Sources */,
    49404944                                CDED342F249DDE0E0002AE7A /* AudioRoutingArbitration.mm in Sources */,
    … …  
    50175021                                7CCE7EEA1A411AE600447C4C /* DidNotHandleKeyDown.cpp in Sources */,
    50185022                                AD57AC211DA7465B00FF1BDE /* DidRemoveFrameFromHiearchyInPageCache.cpp in Sources */,
     5023                                FEC2A85424CE975F00ADBC35 /* DisallowVMEntry.cpp in Sources */,
    50195024                                73BD731823A846500020F450 /* DisplayName.mm in Sources */,
    50205025                                518EE51B20A78D0000E024F3 /* DoAfterNextPresentationUpdateAfterCrash.mm in Sources */,
    … …  
    52485253                                83A2CFDE2481B3520018B2D3 /* ProcessSuspension.mm in Sources */,
    52495254                                518C1153205B0504001FF4AE /* ProcessSwapOnNavigation.mm in Sources */,
     5255                                FEC2A85624CEB65F00ADBC35 /* PropertySlot.cpp in Sources */,
    52505256                                7C83E0C11D0A652F00FEBCF3 /* ProvisionalURLNotChange.mm in Sources */,
    52515257                                5CFACF65226FD2DC0056C7D0 /* Proxy.mm in Sources */,
    52525258                                041A1E34216FFDBC00789E0A /* PublicSuffix.cpp in Sources */,
    5253                                 FEC2A85424CE975F00ADBC35 /* DisallowVMEntry.cpp in Sources */,
    52545259                                7C83E0C21D0A653500FEBCF3 /* QuickLook.mm in Sources */,
    52555260                                6B4E861C2220A5520022F389 /* RegistrableDomain.cpp in Sources */,
    … …  
    54365441                                5E4B1D2E1D404C6100053621 /* WKScrollViewDelegate.mm in Sources */,
    54375442                                F43E3BBF20DADA1E00A4E7ED /* WKScrollViewTests.mm in Sources */,
    5438                                 FEC2A85624CEB65F00ADBC35 /* PropertySlot.cpp in Sources */,
    54395443                                4628C8E92367ABD100B073F0 /* WKSecurityOrigin.cpp in Sources */,
    54405444                                7CCE7F221A411AE600447C4C /* WKString.cpp in Sources */,

  • trunk/Tools/TestWebKitAPI/Tests/TestWebKitAPI/mac/AppKitSPI.h

    r261320 r265059  
    4040- (void)markedRangeWithCompletionHandler:(void(^)(NSRange markedRange))completionHandler;
    4141- (void)hasMarkedTextWithCompletionHandler:(void(^)(BOOL hasMarkedText))completionHandler;
     42- (void)attributedSubstringForProposedRange:(NSRange)range completionHandler:(void(^)(NSAttributedString *, NSRange actualRange))completionHandler;
    4243@end
    4344

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/FontAttributes.mm

    r242339 r265059  
    338338        EXPECT_EQ(0, [attributes[NSSuperscriptAttributeName] integerValue]);
    339339    }
     340    {
     341        [webView selectElementWithIdentifier:@"eleven"];
     342        NSDictionary *attributes = [webView fontAttributesAfterNextPresentationUpdate];
     343        checkColor(attributes[NSForegroundColorAttributeName], { });
     344        checkColor(attributes[NSBackgroundColorAttributeName], { });
     345        checkFont(attributes[NSFontAttributeName], { "Menlo-Regular", 20 });
     346        checkShadow(attributes[NSShadowAttributeName], { });
     347        checkParagraphStyles(attributes[NSParagraphStyleAttributeName], { NSTextAlignmentRight, { } });
     348        EXPECT_EQ(NSUnderlineStyleNone, [attributes[NSStrikethroughStyleAttributeName] integerValue]);
     349        EXPECT_EQ(NSUnderlineStyleNone, [attributes[NSUnderlineStyleAttributeName] integerValue]);
     350        EXPECT_EQ(0, [attributes[NSSuperscriptAttributeName] integerValue]);
     351    }
    340352}
    341353

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/rich-text-attributes.html

    r236955 r265059  
    3030            <span id="ten" style="font-family: Avenir-Book; font-size: 18px; font-style: oblique;">Ten</span>
    3131        </div>
     32        <div id="eleven" dir="rtl" style="text-align: start; font-family: Menlo; font-size: 20px;">Eleven</div>
    3233    </div>
    3334</body>
Note: See TracChangeset for help on using the changeset viewer.