[CP] Fix style_manager_test for Firefox (#181084)#181204
Conversation
Fixes style_manager_test.dart for Firefox by 1. changing the CSS to explicitly set `outline: rgb(0, 0, 0) none 0px` 2. actually focusing the element before checking it's computed style Fixes flutter#180940 - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
github-actions
bot
added
engine
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes a test for Firefox related to CSS outline styles. The changes involve making the CSS more explicit and correcting the test logic to properly focus the element before checking its style. I've suggested an improvement to the test to make it more robust by checking individual CSS properties instead of relying on the browser-specific string representation of the outline shorthand property.
| final expected = isFirefox ? 'rgb(0, 0, 0) 0px' : 'rgb(0, 0, 0) none 0px'; | ||
|
|
||
| // Focus the element. | ||
| flutterViewElement.focusWithoutScroll(); | ||
| final String got = domWindow.getComputedStyle(flutterViewElement).outline; | ||
|
|
||
| expect(got, expected); |
There was a problem hiding this comment.
To make this test more robust and avoid browser-specific string comparisons for the outline shorthand property, it's better to check the individual outline-* properties directly. This removes the need for the isFirefox conditional and makes the test less brittle to changes in how browsers serialize the computed style.
// Focus the element.
flutterViewElement.focusWithoutScroll();
final DomCSSStyleDeclaration style = domWindow.getComputedStyle(flutterViewElement);
// Check the computed style properties individually to make the test more robust
// against different browser serializations of the 'outline' shorthand.
expect(style.getPropertyValue('outline-style'), 'none');
expect(style.getPropertyValue('outline-width'), '0px');
expect(style.getPropertyValue('outline-color'), 'rgb(0, 0, 0)');
jtmcdole
left a comment
jtmcdole
left a comment
There was a problem hiding this comment.
approved for "engine" - need to get release engineer approval
reidbaker
added
the
autosubmit
auto-submit
bot
merged commit a430f66
into
flutter:flutter-3.41-candidate.0
3 participants
Fixes style_manager_test.dart for Firefox by
outline: rgb(0, 0, 0) none 0pxFixes #181203
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for
GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Contributor Guide:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md test-exempt:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests Flutter Style Guide:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md Features we expect every widget to implement:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement CLA: https://cla.developers.google.com/
flutter/tests: https://github.com/flutter/tests
breaking change policy:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes Discord:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md Data Driven Fixes:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.