OPNsense Forum
English Forums => Development and Code Review => Topic started by: NOYB on December 14, 2017, 02:12:25 am
-
It's not that difficult to do in most cases and it prevents breakage.
Symptom:
Can't edit existing rfc2136 clients with IE11.
Cause:
Invalid HTML.
The element button must not appear as a descendant of the a element.
Page:
services_rfc2136.php
HTML Validation:
https://validator.w3.org/
Errors:
Error: A meta element with an http-equiv attribute whose value is X-UA-Compatible must have a content attribute with the value IE=edge.
Error: Duplicate ID __opnsense_csrf.
Error: The element button must not appear as a descendant of the a element.
Specification:
http://w3c.github.io/html/textlevel-semantics.html#the-a-element
"Allowed ARIA role attribute values: link (default - do not set), button, checkbox, radio, switch, tab or treeitem"
"The <a> element may be wrapped around entire paragraphs, lists, tables, and so forth, even entire sections, so long as there is no interactive content within (e.g., buttons or other links)."
-
Hi and welcome,
Let's take this step by step as I don't have an IE 11 to test.
https://github.com/opnsense/core/commit/efb7013ae5a
# opnsense-patch efb7013ae5a
This should make the __opnsense_csrf error disappear, but it needs peer review before inclusion. Although this would have been happening on most static PHP pages for the better part of this year, unless this is a new bug in a recent IE 11.
Bugs are best recorded on GitHub, along with a note for which version of OPNsense and plugin that you are using.
Cheers,
Franco
PS: A good first impression makes all the difference in the world. :)
-
Well that is one of the validation errors. But the one that causes show stopper breakage is the "button" element as descendant of "a" element.
It's pretty simple to fix. Half of it is already done in the DynDNS page.
The other half is just to implement delete anchor in the same way.
Bugs are best recorded on GitHub, along with a note for which version of OPNsense and plugin that you are using.
Was looking for a bug tracker db. GitHub works too though.
re: "A good first impressions make all the difference in the world."
My first impression of most software projects is not good. Too little attention to detail, quality, security, etc. Seems most devs are more interested in playing with cool new features stuff and quantity over quality.
So Yup, just by virtue of being a software project it's starting off in a hole with me. ;)
This bug aside. I saw a "FTR; First Time Right" statement here somewhere. Hope that really is a practice philosophy.
-
Well that is one of the validation errors. But the one that causes show stopper breakage is the "button" element as descendant of "a" element.
I said I don't have IE 11 so I'm at your mercy here. That particular piece of intel I did not have. More so, Chrome does not render the page in a way that this error holds true for me. Maybe I'm doing something wrong here, but still it would help if all the info is presented here in a manner that the line of code can be identified and fixed without going through a couple of iterations.
The other half is just to implement delete anchor in the same way.
I honestly don't know what the context is here, see above. It may seem obvious to you but I'm asking you to be more clear for the reasons already stated above. :)
My first impression of most software projects is not good. Too little attention to detail, quality, security, etc. Seems most devs are more interested in playing with cool new features stuff and quantity over quality.
So Yup, just by virtue of being a software project it's starting off in a hole with me. ;)
This bug aside. I saw a "FTR; First Time Right" statement here somewhere. Hope that really is a practice philosophy.
Then please suspend judgement for a page we didn't originally write, parked in plugins because it receives virtually no maintenance from outside contributors that go through all pages and submit XML cleanups. We don't have that luxury here. We wrote the MVC page framework to make this less cumbersome and migrating everything for the sake of migration is an unfruitful task.
Thanks,
Franco
-
Provided the page.
Provided the HTML validation errors and the validator used.
Provided the HTML spec.
Provided description of why the HTML is out of compliance with the spec.
It's trivial to find and fix from that info alone. Even without access to IE.
Hint: There are only 2 anchor tags with button element descendant. Those are the issue.
Fix: Simply fashion those 2 anchor tags without a button, or other invalid descendant. i.e. in compliance with HTML spec.
You have the necessary information to fix. It's your call.
As for test environment. It should include IE.
As for suspending judgment on code you didn't write; You included it without adequate test.
I'm not really an ogre. I just play one on the outerweb. ;)
My goal isn't to bash or beat anyone over the head. But to set a high bar of expectation for developers, project managers, etc. to produce high quality product.
-
What is your goal here? I don't want to argue, so I'm skipping ahead as I am curious.
I'll say again, without the code line and being unable to reproduce I can't possibly fix it. If that's what you want to prove, you don't have to because I said so myself. :)
Cheers,
Franco
-
Aha, I am terribly sorry for having mistaken "unable to edit" as services_rfc2136_edit.php
-
https://github.com/opnsense/plugins/commit/b6a11cc67
# opnsense-patch -c plugins b6a11cc67
Many thanks for your report! :)
Cheers,
Franco
-
Yeah now we're talkn'.
Way to for stretch out for the high bar. :)
Thanks.
-
Totally OT: WOW, what a debut! :-D
Wellcome to the other side! Please don't bring with you the special "attitude" found in the other forum. Christmas coming! o<:-)
-
I am not apologetic for having high standards regarding attention to detail, quality and security. Three things I highly value.
Did not open this thread to pick a fight or start an argument. I experienced a show stopper breakage issue as a direct result of HTML specification violation (non compliance). A pet peeve. i.e. attention to detail and quality.
Provided the necessary information for it to be fixed. Would loved for it to have been left at that and fixed.
When it comes to attention to detail, quality and security, yes I have an "attitude". I prefer the term "high standard".
When I raise an issue like this it is not for the purpose of bashing. But rather to promote an expectation of adherence to specifications for the purpose of improving the product quality. My hope would be that this is not seen as merely being specific to this instance and forgotten about. But rather for specification adherence to be more of a development focus overall going forward. Which is largely why I posted it in the development forum with this subject question.
In the electronics manufacturing industry years ago there was a concept of testing in quality. Then came along a TV manufacture that turn that concept on it's head. Instead of testing in quality they designed in quality and stopped testing their TV's.
A similar approach is what I'd like to see in software development. Designing in quality by adhering to specifications to significantly reduce the testing necessary and uncaught bugs. When HTML specification is met, if it works in one browser it will near certainly work in other mainstream browsers.
Most of the issues of "works in browser A but not B" vanish when HTML specification is adhered too.
In the end Franco came through and made some progress on reducing that good first impression deficit. ;)
Thank you.
P.S. Since you went OT. I'll be tolerant of you if you are tolerant of me. Or as I sometimes like to challenge people with; "Why are you so intolerant of my intolerance? Don't you think that is hypocritical?"
-
I am not apologetic for having high standards regarding attention to detail, quality and security. Three things I highly value.
Me neither. Though you may way to reevaluate the fact that the subject line would have been a help to the probable contributor by stating "IE 11 edit button in RFC 2136 plugin version 1.4 not working".
High quality code does not equal rough methods to enforce it. That's something that I will not be apologetic for, too. :)
Cheers,
Franco
-
"Why are you so intolerant of my intolerance? Don't you think that is hypocritical?"
Simply answer, as a father of two: Because it ends in better humans. Intolerance is treatabel (better early in live, as some people turn orange otherwise...). And if there have to be discussions on real issues: Let there be discussions! But there is nothing better in live than good manners in hard, focused discussions on real issues. ;-)
Again: Christmas coming soon. Has to tell us all something... :-)
-
Was too late for the initial 17.7.10, but now the plugins have been refreshed and this was hotfixed.
Merry Christmas! as chemlud would say :)