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

CRV2_AppThreatModeling needs updates #317

Closed
kingthorin opened this issue Nov 9, 2020 · 6 comments · Fixed by #459
Closed

CRV2_AppThreatModeling needs updates #317

kingthorin opened this issue Nov 9, 2020 · 6 comments · Fixed by #459
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@kingthorin
Copy link
Contributor

kingthorin commented Nov 9, 2020

https://github.com/OWASP/www-community/blob/master/pages/CRV2_AppThreatModeling.md

Contains a bunch of HTML based tables that need attention. They did not auto-migrate well and at a certain point break the rendering of the majority of the page.

@kingthorin kingthorin added good first issue Good for newcomers help wanted Extra attention is needed labels Nov 9, 2020
@ADubhlaoich
Copy link
Contributor

I can take a shot at this; is retaining the original "look" of the tables important?

Converting to pure markdown should be easy enough, and I'll also do a once-over on the page itself to see what could be improved.

@kingthorin
Copy link
Contributor Author

To be honest I haven't dug into the content much. I doubt the "look" of the table mattered much.

The original is here: https://wiki.owasp.org/index.php/CRV2_AppThreatModeling If that helps.

@ADubhlaoich
Copy link
Contributor

ADubhlaoich commented Dec 4, 2020

As an update to this, there is no clean way to convert straight HTML to Markdown as Markdown tables do not have any "column span" (colspan) arguments for making clear headers in its syntax, and the format for MediaWiki is also messy - hence why the original author likely embedded HTML onto the page.

I am likely going to have to rewrite this entire page to make it readable, as the new website also has different display properties to the original wiki, such as its body width and font size. I'll get the images linked/working properly then start revising the page from the top down.

@ADubhlaoich
Copy link
Contributor

I'm cleaning my hands of this; I've decided that redoing the entire page is far too much of my time, though someone can pick up where I left off if they want to approach it similarly.

I never quite figured out exactly what chunk of raw HTML breaks the page, but converting the tables to Markdown is the quick (Albeit messy) fix for this issue; do note as mentioned above, there is no convention for a multiple-column spanning table heading in Markdown. Even so, rewrites are still necessary - Markdown does not play well with the text in some of the tables, and even my best efforts to reduce text on tables were still distorted once rendered.

My apologies.

@ADubhlaoich ADubhlaoich removed their assignment Jan 20, 2021
@kingthorin
Copy link
Contributor Author

@ADubhlaoich totally understand, thanks for taking a crack at it.

@victoriadrake
Copy link
Contributor

I'll take this.

victoriadrake added a commit to victoriadrake/www-community that referenced this issue Aug 14, 2021
- Combine three pages covering the same topic
- Rename, update, correct Threat Modeling Process (previously
CRV2_AppThreatModeling)

Close OWASP#317
victoriadrake added a commit to victoriadrake/www-community that referenced this issue Aug 14, 2021
- Combine three pages covering the same topic
- Rename, update, correct Threat Modeling Process (previously
CRV2_AppThreatModeling)

Close OWASP#317
kingthorin pushed a commit that referenced this issue Aug 16, 2021
* Update README

- Add instructions for contributors and further guidance

* Fix formatting and images

* Update Threat Modeling pages

- Combine three pages covering the same topic
- Rename, update, correct Threat Modeling Process (previously
CRV2_AppThreatModeling)

Close #317
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants