Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[web] Improve line breaker test exceptions #37244

Merged
merged 5 commits into from
Nov 4, 2022
Merged

[web] Improve line breaker test exceptions #37244

merged 5 commits into from
Nov 4, 2022

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Nov 2, 2022 •
edited
Loading

Instead of hardcoding test exceptions, use regular expressions to modify the desired tests.

This PR also makes the line breaker always break after a sequence of spaces, even if the spec says otherwise.

Why are you doing this now?

Because soon I'm introducing the usage of Chrome's v8BreakIterator which needs its own set of exceptions. So it made sense to cleanup the existing exception in preparation for this.

Related to flutter/flutter#112820

@mdebbar mdebbar requested a review from yjbanov November 2, 2022 15:46
@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Nov 2, 2022
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

yjbanov
yjbanov approved these changes Nov 4, 2022
View reviewed changes
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

lib/web_ui/lib/src/engine/text/line_breaker.dart
baseOfSpaceSequence == LineCharProperty.CP) &&
//
// The above is a quote from unicode.org. In our implementation, we did the
// following modification: Allow breaks when there are spaces.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have details about why we modified it documented anywhere? Maybe expand on it here a bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. We modified it to match Chrome's behavior (which makes more sense to me than the unicode spec in these cases). I'll explain it in the comment.

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 4, 2022
@auto-submit auto-submit bot merged commit 805b145 into flutter:main Nov 4, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 4, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 4, 2022
@engine-flutter-autoroll
805b145d7 [web] Improve line breaker test exceptions (flutter/engine#…
7eb343a 
…37244)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 4, 2022
@engine-flutter-autoroll
805b145d7 [web] Improve line breaker test exceptions (flutter/engine#…
77d6ecb 
…37244) (#114688)
sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this pull request Nov 4, 2022
flutter: 805b145d7 [web] Improve line breaker test exceptions (flutte…
067c7b3 
…r/engine#37244) (#114688)

Commit: 77d6ecb83b6ca8e51eaa7c5579cb975d09693552
This was referenced Nov 5, 2022
Merged
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 5, 2022
@engine-flutter-autoroll
77d6ecb 805b145d7 [web] Improve line breaker test exceptions (flutter…
34f07e4 
…/engine#37244) (flutter/flutter#114688)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Nov 5, 2022
@engine-flutter-autoroll
77d6ecb 805b145d7 [web] Improve line breaker test exceptions (flutter…
d68ed00 
…/engine#37244) (flutter/flutter#114688)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 5, 2022
@engine-flutter-autoroll
Roll Flutter from 55e8cd1 to e0e7027 (20 revisions) (#2782)
bf3328c 
* 77d6ecb 805b145d7 [web] Improve line breaker test exceptions (flutter/engine#37244) (flutter/flutter#114688)

* 5233fd9 Roll Plugins from 5c11747 to a279b9d (6 revisions) (flutter/flutter#114689)

* e1166e4 287a3ab59 Roll Skia from c901cb6ae66f to 1272b520c082 (1 revision) (flutter/engine#37314) (flutter/flutter#114691)

* 48457d7 WidgetController.startGesture trackpad support (flutter/flutter#114631)

* c1d2b85 Change some required nullable parameters in tool to non-null (flutter/flutter#114115)

* cfb2f15 Roll Flutter Engine from 287a3ab59269 to e947833ceb5b (2 revisions) (flutter/flutter#114696)

* 80bf355 Support keyboard selection in SelectabledRegion (flutter/flutter#112584)

* be83c98 5117e858e Fix typo in overlay platform view iOS test (flutter/engine#37262) (flutter/flutter#114705)

* 6ea01e6 Roll Flutter Engine from 5117e858ed1e to 6950689ed775 (3 revisions) (flutter/flutter#114710)

* b187bc4 Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (flutter/flutter#114637)

* 01507ba Revert "Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (#114637)" (flutter/flutter#114715)

* 91a56cf roll packages (flutter/flutter#114366)

* d68af26 Roll Flutter Engine from 6950689ed775 to 0933ca485a07 (3 revisions) (flutter/flutter#114714)

* 4fd0a92 Remove auto_cleanup of xcode caches. (flutter/flutter#114719)

* 96f9ca8 Roll Flutter Engine from 0933ca485a07 to 28e1ea0cc5b1 (4 revisions) (flutter/flutter#114720)

* 1aada6f [New Feature]Support mouse wheel event on the scrollbar widget (flutter/flutter#109659)

* 6265610 Roll Flutter Engine from 28e1ea0cc5b1 to ba390f2a7b7d (3 revisions) (flutter/flutter#114728)

* 140a57e Roll Flutter Engine from ba390f2a7b7d to 223a485ceb02 (3 revisions) (flutter/flutter#114732)

* 06ded49 Roll Flutter Engine from 223a485ceb02 to cf56eb5565e2 (3 revisions) (flutter/flutter#114735)

* e0e7027 9c45b0ebe Roll Fuchsia Mac SDK from jAKH68TYoKUA5HNS2... to Ua8Jtf8Zka9uxIVdl... (flutter/engine#37345) (flutter/flutter#114746)
auto-submit bot pushed a commit to flutter/plugins that referenced this pull request Nov 5, 2022
@engine-flutter-autoroll
Roll Flutter from 55e8cd1 to e0e7027 (20 revisions) (#6673)
f04ef73 
* 77d6ecb 805b145d7 [web] Improve line breaker test exceptions (flutter/engine#37244) (flutter/flutter#114688)

* 5233fd9 Roll Plugins from 5c11747 to a279b9d (6 revisions) (flutter/flutter#114689)

* e1166e4 287a3ab59 Roll Skia from c901cb6ae66f to 1272b520c082 (1 revision) (flutter/engine#37314) (flutter/flutter#114691)

* 48457d7 WidgetController.startGesture trackpad support (flutter/flutter#114631)

* c1d2b85 Change some required nullable parameters in tool to non-null (flutter/flutter#114115)

* cfb2f15 Roll Flutter Engine from 287a3ab59269 to e947833ceb5b (2 revisions) (flutter/flutter#114696)

* 80bf355 Support keyboard selection in SelectabledRegion (flutter/flutter#112584)

* be83c98 5117e858e Fix typo in overlay platform view iOS test (flutter/engine#37262) (flutter/flutter#114705)

* 6ea01e6 Roll Flutter Engine from 5117e858ed1e to 6950689ed775 (3 revisions) (flutter/flutter#114710)

* b187bc4 Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (flutter/flutter#114637)

* 01507ba Revert "Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (#114637)" (flutter/flutter#114715)

* 91a56cf roll packages (flutter/flutter#114366)

* d68af26 Roll Flutter Engine from 6950689ed775 to 0933ca485a07 (3 revisions) (flutter/flutter#114714)

* 4fd0a92 Remove auto_cleanup of xcode caches. (flutter/flutter#114719)

* 96f9ca8 Roll Flutter Engine from 0933ca485a07 to 28e1ea0cc5b1 (4 revisions) (flutter/flutter#114720)

* 1aada6f [New Feature]Support mouse wheel event on the scrollbar widget (flutter/flutter#109659)

* 6265610 Roll Flutter Engine from 28e1ea0cc5b1 to ba390f2a7b7d (3 revisions) (flutter/flutter#114728)

* 140a57e Roll Flutter Engine from ba390f2a7b7d to 223a485ceb02 (3 revisions) (flutter/flutter#114732)

* 06ded49 Roll Flutter Engine from 223a485ceb02 to cf56eb5565e2 (3 revisions) (flutter/flutter#114735)

* e0e7027 9c45b0ebe Roll Fuchsia Mac SDK from jAKH68TYoKUA5HNS2... to Ua8Jtf8Zka9uxIVdl... (flutter/engine#37345) (flutter/flutter#114746)
IVLIVS-III pushed a commit to IVLIVS-III/flutter_plugins_fork that referenced this pull request Nov 11, 2022
@engine-flutter-autoroll @IVLIVS-III
Roll Flutter from 55e8cd1 to e0e7027 (20 revisions) (flutter#6673)
ad466d6 
* 77d6ecb 805b145d7 [web] Improve line breaker test exceptions (flutter/engine#37244) (flutter/flutter#114688)

* 5233fd9 Roll Plugins from 5c11747 to a279b9d (6 revisions) (flutter/flutter#114689)

* e1166e4 287a3ab59 Roll Skia from c901cb6ae66f to 1272b520c082 (1 revision) (flutter/engine#37314) (flutter/flutter#114691)

* 48457d7 WidgetController.startGesture trackpad support (flutter/flutter#114631)

* c1d2b85 Change some required nullable parameters in tool to non-null (flutter/flutter#114115)

* cfb2f15 Roll Flutter Engine from 287a3ab59269 to e947833ceb5b (2 revisions) (flutter/flutter#114696)

* 80bf355 Support keyboard selection in SelectabledRegion (flutter/flutter#112584)

* be83c98 5117e858e Fix typo in overlay platform view iOS test (flutter/engine#37262) (flutter/flutter#114705)

* 6ea01e6 Roll Flutter Engine from 5117e858ed1e to 6950689ed775 (3 revisions) (flutter/flutter#114710)

* b187bc4 Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (flutter/flutter#114637)

* 01507ba Revert "Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (#114637)" (flutter/flutter#114715)

* 91a56cf roll packages (flutter/flutter#114366)

* d68af26 Roll Flutter Engine from 6950689ed775 to 0933ca485a07 (3 revisions) (flutter/flutter#114714)

* 4fd0a92 Remove auto_cleanup of xcode caches. (flutter/flutter#114719)

* 96f9ca8 Roll Flutter Engine from 0933ca485a07 to 28e1ea0cc5b1 (4 revisions) (flutter/flutter#114720)

* 1aada6f [New Feature]Support mouse wheel event on the scrollbar widget (flutter/flutter#109659)

* 6265610 Roll Flutter Engine from 28e1ea0cc5b1 to ba390f2a7b7d (3 revisions) (flutter/flutter#114728)

* 140a57e Roll Flutter Engine from ba390f2a7b7d to 223a485ceb02 (3 revisions) (flutter/flutter#114732)

* 06ded49 Roll Flutter Engine from 223a485ceb02 to cf56eb5565e2 (3 revisions) (flutter/flutter#114735)

* e0e7027 9c45b0ebe Roll Fuchsia Mac SDK from jAKH68TYoKUA5HNS2... to Ua8Jtf8Zka9uxIVdl... (flutter/engine#37345) (flutter/flutter#114746)
schwa423 pushed a commit to schwa423/engine that referenced this pull request Nov 16, 2022
@mdebbar @schwa423
[web] Improve line breaker test exceptions (flutter#37244)
26651d4 
* [web] Improve line breaker test exceptions

* always break after spaces

* explain reason for modifications
percula pushed a commit to percula/packages that referenced this pull request Nov 17, 2022
@engine-flutter-autoroll @percula
Roll Flutter from 55e8cd1 to e0e7027 (20 revisions) (flutter#2782)
0e341bc 
* 77d6ecb 805b145d7 [web] Improve line breaker test exceptions (flutter/engine#37244) (flutter/flutter#114688)

* 5233fd9 Roll Plugins from 5c11747 to a279b9d (6 revisions) (flutter/flutter#114689)

* e1166e4 287a3ab59 Roll Skia from c901cb6ae66f to 1272b520c082 (1 revision) (flutter/engine#37314) (flutter/flutter#114691)

* 48457d7 WidgetController.startGesture trackpad support (flutter/flutter#114631)

* c1d2b85 Change some required nullable parameters in tool to non-null (flutter/flutter#114115)

* cfb2f15 Roll Flutter Engine from 287a3ab59269 to e947833ceb5b (2 revisions) (flutter/flutter#114696)

* 80bf355 Support keyboard selection in SelectabledRegion (flutter/flutter#112584)

* be83c98 5117e858e Fix typo in overlay platform view iOS test (flutter/engine#37262) (flutter/flutter#114705)

* 6ea01e6 Roll Flutter Engine from 5117e858ed1e to 6950689ed775 (3 revisions) (flutter/flutter#114710)

* b187bc4 Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (flutter/flutter#114637)

* 01507ba Revert "Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (#114637)" (flutter/flutter#114715)

* 91a56cf roll packages (flutter/flutter#114366)

* d68af26 Roll Flutter Engine from 6950689ed775 to 0933ca485a07 (3 revisions) (flutter/flutter#114714)

* 4fd0a92 Remove auto_cleanup of xcode caches. (flutter/flutter#114719)

* 96f9ca8 Roll Flutter Engine from 0933ca485a07 to 28e1ea0cc5b1 (4 revisions) (flutter/flutter#114720)

* 1aada6f [New Feature]Support mouse wheel event on the scrollbar widget (flutter/flutter#109659)

* 6265610 Roll Flutter Engine from 28e1ea0cc5b1 to ba390f2a7b7d (3 revisions) (flutter/flutter#114728)

* 140a57e Roll Flutter Engine from ba390f2a7b7d to 223a485ceb02 (3 revisions) (flutter/flutter#114732)

* 06ded49 Roll Flutter Engine from 223a485ceb02 to cf56eb5565e2 (3 revisions) (flutter/flutter#114735)

* e0e7027 9c45b0ebe Roll Fuchsia Mac SDK from jAKH68TYoKUA5HNS2... to Ua8Jtf8Zka9uxIVdl... (flutter/engine#37345) (flutter/flutter#114746)
adam-harwood pushed a commit to adam-harwood/flutter_plugins that referenced this pull request Nov 21, 2022
@engine-flutter-autoroll
Roll Flutter from 55e8cd1 to e0e7027 (20 revisions) (flutter#6673)
854dd32 
* 77d6ecb 805b145d7 [web] Improve line breaker test exceptions (flutter/engine#37244) (flutter/flutter#114688)

* 5233fd9 Roll Plugins from 5c11747 to a279b9d (6 revisions) (flutter/flutter#114689)

* e1166e4 287a3ab59 Roll Skia from c901cb6ae66f to 1272b520c082 (1 revision) (flutter/engine#37314) (flutter/flutter#114691)

* 48457d7 WidgetController.startGesture trackpad support (flutter/flutter#114631)

* c1d2b85 Change some required nullable parameters in tool to non-null (flutter/flutter#114115)

* cfb2f15 Roll Flutter Engine from 287a3ab59269 to e947833ceb5b (2 revisions) (flutter/flutter#114696)

* 80bf355 Support keyboard selection in SelectabledRegion (flutter/flutter#112584)

* be83c98 5117e858e Fix typo in overlay platform view iOS test (flutter/engine#37262) (flutter/flutter#114705)

* 6ea01e6 Roll Flutter Engine from 5117e858ed1e to 6950689ed775 (3 revisions) (flutter/flutter#114710)

* b187bc4 Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (flutter/flutter#114637)

* 01507ba Revert "Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (#114637)" (flutter/flutter#114715)

* 91a56cf roll packages (flutter/flutter#114366)

* d68af26 Roll Flutter Engine from 6950689ed775 to 0933ca485a07 (3 revisions) (flutter/flutter#114714)

* 4fd0a92 Remove auto_cleanup of xcode caches. (flutter/flutter#114719)

* 96f9ca8 Roll Flutter Engine from 0933ca485a07 to 28e1ea0cc5b1 (4 revisions) (flutter/flutter#114720)

* 1aada6f [New Feature]Support mouse wheel event on the scrollbar widget (flutter/flutter#109659)

* 6265610 Roll Flutter Engine from 28e1ea0cc5b1 to ba390f2a7b7d (3 revisions) (flutter/flutter#114728)

* 140a57e Roll Flutter Engine from ba390f2a7b7d to 223a485ceb02 (3 revisions) (flutter/flutter#114732)

* 06ded49 Roll Flutter Engine from 223a485ceb02 to cf56eb5565e2 (3 revisions) (flutter/flutter#114735)

* e0e7027 9c45b0ebe Roll Fuchsia Mac SDK from jAKH68TYoKUA5HNS2... to Ua8Jtf8Zka9uxIVdl... (flutter/engine#37345) (flutter/flutter#114746)
shogohida pushed a commit to shogohida/flutter that referenced this pull request Dec 7, 2022
@engine-flutter-autoroll @shogohida
805b145d7 [web] Improve line breaker test exceptions (flutter/engine#…
d536981 
…37244) (flutter#114688)
@mdebbar mdebbar deleted the line_break_test_helper branch January 17, 2023 18:24
mauricioluz pushed a commit to mauricioluz/plugins that referenced this pull request Jan 26, 2023
@engine-flutter-autoroll @mauricioluz
Roll Flutter from 55e8cd1 to e0e7027 (20 revisions) (flutter#6673)
252434f 
* 77d6ecb 805b145d7 [web] Improve line breaker test exceptions (flutter/engine#37244) (flutter/flutter#114688)

* 5233fd9 Roll Plugins from 5c11747 to a279b9d (6 revisions) (flutter/flutter#114689)

* e1166e4 287a3ab59 Roll Skia from c901cb6ae66f to 1272b520c082 (1 revision) (flutter/engine#37314) (flutter/flutter#114691)

* 48457d7 WidgetController.startGesture trackpad support (flutter/flutter#114631)

* c1d2b85 Change some required nullable parameters in tool to non-null (flutter/flutter#114115)

* cfb2f15 Roll Flutter Engine from 287a3ab59269 to e947833ceb5b (2 revisions) (flutter/flutter#114696)

* 80bf355 Support keyboard selection in SelectabledRegion (flutter/flutter#112584)

* be83c98 5117e858e Fix typo in overlay platform view iOS test (flutter/engine#37262) (flutter/flutter#114705)

* 6ea01e6 Roll Flutter Engine from 5117e858ed1e to 6950689ed775 (3 revisions) (flutter/flutter#114710)

* b187bc4 Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (flutter/flutter#114637)

* 01507ba Revert "Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (#114637)" (flutter/flutter#114715)

* 91a56cf roll packages (flutter/flutter#114366)

* d68af26 Roll Flutter Engine from 6950689ed775 to 0933ca485a07 (3 revisions) (flutter/flutter#114714)

* 4fd0a92 Remove auto_cleanup of xcode caches. (flutter/flutter#114719)

* 96f9ca8 Roll Flutter Engine from 0933ca485a07 to 28e1ea0cc5b1 (4 revisions) (flutter/flutter#114720)

* 1aada6f [New Feature]Support mouse wheel event on the scrollbar widget (flutter/flutter#109659)

* 6265610 Roll Flutter Engine from 28e1ea0cc5b1 to ba390f2a7b7d (3 revisions) (flutter/flutter#114728)

* 140a57e Roll Flutter Engine from ba390f2a7b7d to 223a485ceb02 (3 revisions) (flutter/flutter#114732)

* 06ded49 Roll Flutter Engine from 223a485ceb02 to cf56eb5565e2 (3 revisions) (flutter/flutter#114735)

* e0e7027 9c45b0ebe Roll Fuchsia Mac SDK from jAKH68TYoKUA5HNS2... to Ua8Jtf8Zka9uxIVdl... (flutter/engine#37345) (flutter/flutter#114746)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yjbanov yjbanov yjbanov approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App needs tests platform-web Code specifically for the web engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mdebbar @yjbanov