[Webkit-unassigned] [Bug 55396] [Mac] Make "=?UTF-8?Q?Change=20back=20to=20=E2=80=A6?=" contextual menu item work with new autocorrection.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=55396


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #84102|review?, commit-queue?      |review+, commit-queue+
               Flag|                            |




--- Comment #2 from Darin Adler <darin at apple.com>  2011-02-28 13:52:23 PST ---
(From update of attachment 84102)
View in context: https://bugs.webkit.org/attachment.cgi?id=84102&action=review

> Source/WebCore/editing/Editor.cpp:2496
> +    String replacement = plainText(selection.get());
> +    client()->recordAutocorrectionResponse(EditorClient::AutocorrectionReverted, replacedString, replacement);

You could write this without the local variable. The local variable does provide a bit of documentation, so I guess it could go either way.

> Source/WebCore/rendering/InlineTextBox.cpp:1098
> +            case DocumentMarker::Replacement:
> +                computeRectForReplacementMarker(marker, style, font);
> +                break;

Is there a way to test this?

> Source/WebCore/rendering/InlineTextBox.cpp:1100
>              default:
>                  ASSERT_NOT_REACHED();

If this default wasn’t here, then the compiler would have warned about the fact that the enum value was not handled, so we might have spotted this earlier. Generally speaking, it’s best to structure things so that switches based on enums do not have a default case. Not sure how easy that would be in this case, without sacrificing the “bad enum value” assertion.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.