Skip to content

/ servo

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

Introduce ElementCSSInlineStyle for SVGElement #26789

Merged
merged 1 commit into from Jun 4, 2020
Merged

Introduce ElementCSSInlineStyle for SVGElement #26789

merged 1 commit into from Jun 4, 2020
+49 −2

Conversation

@CYBAI
Copy link
Collaborator

CYBAI commented Jun 4, 2020 •
edited

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #26777 and fix #26032 and fix #21990
  • There are tests for these changes
@highfive
Copy link

highfive commented Jun 4, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/svgelement.rs, components/script/dom/create.rs, components/script/dom/webidls/SVGElement.webidl
  • @KiChjang: components/script/dom/svgelement.rs, components/script/dom/create.rs, components/script/dom/webidls/SVGElement.webidl
@highfive
Copy link

highfive commented Jun 4, 2020

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@CYBAI
Introduce ElementCSSInlineStyle for SVGElement
Loading status checks…
c561a3e
@CYBAI CYBAI force-pushed the CYBAI:svgelement-style branch from 80d58a9 to c561a3e Jun 4, 2020
@CYBAI CYBAI changed the title [WIP] Introduce ElementCSSInlineStyle for SVGElement Introduce ElementCSSInlineStyle for SVGElement Jun 4, 2020
@CYBAI CYBAI marked this pull request as ready for review Jun 4, 2020
@CYBAI
Copy link
Collaborator Author

CYBAI commented Jun 4, 2020

I haven't tried to build an apk and run it in an Android phone or emulator but I think this should also fix #21990 🤔
@CYBAI
Copy link
Collaborator Author

CYBAI commented Jun 4, 2020

@bors-servo try=wpt
bors-servo added a commit that referenced this pull request Jun 4, 2020
@bors-servo
Auto merge of #26789 - CYBAI:svgelement-style, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
56e91fa 
Introduce ElementCSSInlineStyle for SVGElement

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #26777 and fix #26032 and fix #21990
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2020

⌛ Trying commit c561a3e with merge 56e91fa...
@CYBAI
CYBAI reviewed Jun 4, 2020
View changes
components/script/dom/svgelement.rs
@@ -13,9 +18,18 @@ use style::element_state::ElementState;
#[dom_struct]
pub struct SVGElement {
element: Element,
style_decl: MutNullableDom<CSSStyleDeclaration>,

This comment has been minimized.

Sign in to view
@CYBAI

CYBAI Jun 4, 2020

Author Collaborator

also cc @emilio

I basically refer to HTMLElement to add this field. Does this make sense?

This comment has been minimized.

Sign in to view
@emilio

emilio Jun 4, 2020

Member

It does, though maybe it should be moved down to Element, even if not web-exposed there, because that's where style_attribute lives. Anyhow no big deal.
@jdm
jdm approved these changes Jun 4, 2020
View changes
Copy link
Member

jdm left a comment

This looks fine to me if tests pass!
@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2020

💔 Test failed - status-taskcluster
@jdm
Copy link
Member

jdm commented Jun 4, 2020

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2020

📌 Commit c561a3e has been approved by jdm
@highfive highfive assigned jdm and unassigned SimonSapin Jun 4, 2020
@CYBAI
Copy link
Collaborator Author

CYBAI commented Jun 4, 2020

thanks! btw, with enabling svg for tests locally, I can confirm there are some passing tests for SVGElement.style now!
@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2020

⌛ Testing commit c561a3e with merge 59ec134...
@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 59ec134 to master...
@bors-servo bors-servo merged commit 59ec134 into servo:master Jun 4, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@CYBAI CYBAI deleted the CYBAI:svgelement-style branch Jun 5, 2020
@CYBAI CYBAI mentioned this pull request Jun 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilio emilio

@jdm jdm

Assignees

@jdm jdm

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Implement SVGElement.style
Twitter won't load - stuck in endless refresh prompt
mobile.twitter.com displays error page on Android
6 participants
@CYBAI @highfive @bors-servo @jdm @emilio @SimonSapin
You can’t perform that action at this time.