X Tutup
Skip to content

Provide support for SVG foreignObject by adding xhtml namespace#6192

Closed
gionkunz wants to merge 1 commit intoangular:masterfrom
gionkunz:svg-foreign-object
Closed

Provide support for SVG foreignObject by adding xhtml namespace#6192
gionkunz wants to merge 1 commit intoangular:masterfrom
gionkunz:svg-foreign-object

Conversation

@gionkunz
Copy link
Copy Markdown
Contributor

@gionkunz gionkunz commented Dec 31, 2015

This PR enables the use of elements with HTML namespace within SVG foreignObjects.

@gionkunz gionkunz mentioned this pull request Dec 31, 2015
Closed
@vsavkin vsavkin added the action: review The PR is still awaiting reviews from at least one requested reviewer label Feb 1, 2016
@tbosch
Copy link
Copy Markdown
Contributor

tbosch commented Feb 9, 2016

Looks good, although not really standards compliant. The svg docs give this example for using html in svg:

<foreignObject width="100" height="50">
      <!-- XHTML content goes here -->
      <body xmlns="http://www.w3.org/1999/xhtml">
        <p>Here is a paragraph that requires word wrap</p>
      </body>
</foreignObject>

But let's keep this as is for now so that we have at least a working path on which we can improve later if needed...

@tbosch tbosch added pr_state: LGTM action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 9, 2016
@gionkunz
Copy link
Copy Markdown
Contributor Author

gionkunz commented Feb 10, 2016

Well the specification is not quite clear about the contents. Using a body as a root element within the foreignObject is just an example. They are talking about document fragments so I'd assume that we can refer to https://www.w3.org/2003/01/dom2-javadoc/org/w3c/dom/DocumentFragment.html as what is a valid document fragment. I'm using foreignObjects within my library https://github.com/gionkunz/chartist-js and so far all browsers are totally fine with foreignObjects containing no body element. Somehow we'd need to exclude this test for IE however, as they don't support foreignObject at all.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Feb 10, 2016

Merging PR #6192 on behalf of @btford to branch presubmit-btford-pr-6192.

@mhevery mhevery closed this in eb688f2 Feb 10, 2016
@btford btford reopened this Feb 11, 2016
@btford
Copy link
Copy Markdown
Contributor

btford commented Feb 11, 2016

This change needed to be reverted. We can probably merge this again independently, or once #6363 is resolved

@gionkunz
Copy link
Copy Markdown
Contributor Author

gionkunz commented Feb 12, 2016

@btford is there any action required from my side if #6363 is merged?

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Feb 25, 2016

Merging PR #6192 on behalf of @vsavkin to branch presubmit-vsavkin-pr-6192.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Feb 25, 2016

Merging PR #6192 on behalf of @vsavkin to branch presubmit-vsavkin-pr-6192.

@vikerman vikerman removed the action: merge The PR is ready for merge by the caretaker label Mar 3, 2016
@vikerman
Copy link
Copy Markdown
Contributor

vikerman commented Mar 3, 2016

Breaks google3. @tbosch - Here is the link for the failure - https://test.corp.google.com/ui#cl=114446968

Please re-add the merge label once this is resolved.

@tbosch tbosch added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Mar 9, 2016
@tbosch tbosch removed their assignment Mar 9, 2016
@tbosch
Copy link
Copy Markdown
Contributor

tbosch commented Mar 10, 2016

Sorry, don't have time to work on this right now...

@mhevery
Copy link
Copy Markdown
Contributor

mhevery commented May 25, 2016

Hi @gionkunz, sorry for the long delay in getting this in, totally our fault. Can you try to rebase to the laste master?

If we don't get an update in a week, we will close this PR as obsolete.

@mhevery mhevery self-assigned this May 25, 2016
@gionkunz
Copy link
Copy Markdown
Contributor Author

gionkunz commented May 25, 2016

Sure, will do. Cheers

@mhevery
Copy link
Copy Markdown
Contributor

mhevery commented Jun 9, 2016

@gionkunz Any update on the rebase or should we just abandon this PR?

@gionkunz
Copy link
Copy Markdown
Contributor Author

gionkunz commented Jun 10, 2016

@mhevery Sorry I got a bit sidetracked from this issue. Please leave it open. I'll schedule some time for this next week. Cheers

@gionkunz gionkunz closed this Jun 10, 2016
@gionkunz gionkunz reopened this Jun 10, 2016
@vicb
Copy link
Copy Markdown
Contributor

vicb commented Jun 21, 2016

@gionkunz any update ?

@gionkunz
Copy link
Copy Markdown
Contributor Author

gionkunz commented Jun 22, 2016

@vicb @mhevery I'm closing this PR in favor of #9458

@gionkunz gionkunz closed this Jun 22, 2016
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Sep 8, 2019

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@mhevery mhevery

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@gionkunz @tbosch @mary-poppins @btford @vikerman @mhevery @vicb @vsavkin @naomiblack @googlebot
X Tutup