Bug 218020 – Stop gap patch fix for regression in r267329.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Stop gap patch fix for regression in r267329.

[Megan Gardner]

Created attachment 411980 [details]
Patch

[Megan Gardner]

Created attachment 412012 [details]
Patch

[Sam Weinig]

Comment on attachment 412012 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412012&action=review

> Source/WebCore/ChangeLog:7
> +

What was the regression? How does this fix it? Where is the long term solution being tracked?

[Megan Gardner]

Created attachment 412017 [details]
Patch

[Megan Gardner]

Created attachment 412019 [details]
Patch

[Darin Adler]

Comment on attachment 412019 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412019&action=review

Looks good. I really appreciate having the test, which I will keep working and/or refine as I return to the Selection API work.

> Source/WebCore/editing/VisibleSelection.cpp:390
> -    bool shouldUpdateAnchor = m_start != startBeforeAdjustments;
> -    bool shouldUpdateFocus = m_end != endBeforeAdjustments;
> +    bool shouldUpdateAnchor = false;
> +    bool shouldUpdateFocus = false;

Might be good to leave a comment behind here. The fact that these are always false makes the code peculiar for now, but makes sense if you know we intend to change it later.

[Megan Gardner]

Created attachment 412033 [details]
Patch

[Megan Gardner]

Created attachment 412039 [details]
Patch

[Darin Adler]

Comment on attachment 412039 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412039&action=review

> Source/WebCore/editing/VisibleSelection.cpp:389
> +    bool shouldUpdateAnchor = false; // Set to false because of <rdar://problem/69542459>. Can be returned to original logic when this problem is fully fixed.

Probably more kind to open source contributors to cite https://bugs.webkit.org/show_bug.cgi?id=218020 instead of rdar, but clear enough either way. I promise to remove this comment some day either way.

[EWS]

Committed r268847: <https://trac.webkit.org/changeset/268847>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412039 [details].

[Radar WebKit Bug Importer]

<rdar://problem/70553518>

[Darin Adler]

Seems like the test isn’t working well to detect the problem. I rolled the change back and the test still passes!

[Darin Adler]

No, I was running the test wrong. I was confused by the ChangeLog, which lists the wrong directory for the test.