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.

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

Add deserialization steps for web frame and web window #1738

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

whimboo
Copy link
Contributor

@whimboo whimboo commented May 15, 2023

While implementing the serialization of window and frame objects I noticed that there are no deserialization steps available for these two object types.


Preview | Diff

@whimboo whimboo changed the title Add deserialization steps for web frame an web window Add deserialization steps for web frame and web window May 16, 2023
Copy link
Member

@gsnedders gsnedders left a comment

Choose a reason for hiding this comment

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

This seems fine modulo the nit.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@whimboo
Copy link
Contributor Author

whimboo commented May 26, 2023

@jgraham mind reviewing as well? Thanks.

@whimboo
Copy link
Contributor Author

whimboo commented Aug 4, 2023

@mathiasbynens could someone from Google review this addition as well? I'm not sure who to tag given that @sadym-chromium is out at the moment.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
jgraham
jgraham previously requested changes Aug 7, 2023
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@whimboo
Copy link
Contributor Author

whimboo commented Aug 7, 2023

@jgraham fixes for both of your comments have been pushed.

@whimboo
Copy link
Contributor Author

whimboo commented Oct 16, 2023

@jgraham merging this PR is blocked on your review comments. Can you please re-check if you have the time? Thanks.

@whimboo whimboo requested a review from jgraham October 16, 2023 15:56
@whimboo
Copy link
Contributor Author

whimboo commented Oct 16, 2023

@jgraham merging this PR is blocked on your review comments. Can you please re-check if you have the time? Thanks.

Given that the review comments from James have been addressed, maybe @juliandescottes or @lutien can have a final look at the review?

Copy link
Contributor

@lutien lutien left a comment

Choose a reason for hiding this comment

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

James' comments are addressed and I see no other issues.

@whimboo whimboo dismissed jgraham’s stale review October 17, 2023 08:58

Review comments have been addressed and Sasha re-reviewed

@whimboo
Copy link
Contributor Author

whimboo commented Oct 17, 2023

Please note that tests will be added as part of https://bugzilla.mozilla.org/show_bug.cgi?id=1274251.

@whimboo whimboo merged commit ea8ff97 into w3c:master Oct 17, 2023
@whimboo whimboo deleted the windor_frame_deserialize branch October 17, 2023 08:59
github-actions bot added a commit that referenced this pull request Oct 17, 2023
SHA: ea8ff97
Reason: push, by whimboo

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit to xjc90s/webdriver that referenced this pull request Oct 17, 2023
SHA: ea8ff97
Reason: push, by pull[bot]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit to soloinovator/webdriver that referenced this pull request Oct 17, 2023
SHA: ea8ff97
Reason: push, by pull[bot]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit to ZRDKPoWeR/webdriver that referenced this pull request Oct 21, 2023
SHA: ea8ff97
Reason: push, by pull[bot]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants