using common CSS class names on Jasmine DOM elements can cause unintended behavior

[prather-mcs]

I'm a relative new-comer in the field of front-end web dev, and there is no other comp sci / programming field in which I'm more experienced. (But hear me out anyway, ha ha.)

While working on a student project, I noticed something about Jasmine which you might consider a "bug" which causes unintended DOM interactions between Jasmine's part of the DOM and the app-that's-being-tested's part of the DOM.

And this bug does not have to exist at all; you can painlessly avoid it.

So, what is this problem?

Well, I first noticed a specific case of this bug when I saw some of Jasmine's text appearing in one of the app's divs on a student project I was working on — the words "Spec List" and "Failures" appeared on top of what should have been in my app's .menu div. This occurred whenever a specification failed.

I have reproduced a case of this bug here:
http://codepen.io/prather-mcs/pen/bdVrdN
(this Pen is loading all necessary Jasmine assets from here)

When you un-comment the test expect(false).toBe(true);, then you will see the bug happen: the app's CSS for .menu is affecting the position of Jasmine's DOM elements that also have class .menu

I'm sure you want to always keep Jasmine's information down in Jasmine's part of the document.

So that's a specific case where .menu is colliding.

But I think the general problem you would want to fix is the creation by Jasmine of DOM elements with such commonly-used class-names as these:

• .bar
• .menu
• .results
• .summary
• .banner

In summary, the Issue is this:

While none of Jasmine's CSS rules in jasmine.css change how the app appears, style rules in the tested app which affect any class name that jasmine.css also uses can cause Jasmine's DOM content to be styled in un-intended ways, including causing it to jump right out of Jasmine's part of the page and onto the app's part of the page.

---

I would be happy to get credit for fixing this bug -- I would go to the jasmine-html.js code that creates DOM Nodes and ensure that all CSS class names are unique (enough) by giving them slightly new names, such as changing code that would add the class .menu to a DOM Node to actually add the class .jasmine-menu instead. Then I would make corresponding changes to jasmine.css as well.

However, I have not contributed to this project before, and have only been looking into the code-base for the first time this week.

I'm thinking that there are people with seniority and authority who would probably have a better idea of how to name the CSS classes than I have, and they would have a better idea of the best Jasmine app code to write to efficiently implement the fix as well.

With that being said, if you would allow me to write a fix and create a Pull request to fix this bug, I would like to get some credit contributing to open source....

[nertzy]

👍 to prepending jasmine- to the front of all classes and IDs that are injected into the page.

I'd encourage you to give it a good try. If any of the maintainers disagree with any of your choices they can comment on the Pull Request and give feedback, although everything you're saying sounds quite reasonable.

[prather-mcs]

Thanks for the counsel. I was definitely thinking of proceeding to that next step. Told myself I'd wait a week. I have a lot of student work, but the chance to make an accepted commit to a real-world project like Jasmine is probably well worth the time. It does make sense to craft a solution and request pull, because then they can review it and tell me what they really want to see, and then I can do that, and then bam it's done. Thanks again.

edit/update:
I've now (2015-05-09) officially started trying to write a fix, in order to then make a pull request and start a dialog there. Figuring out how / where to implement the fix is more difficult than I first thought. But that's not a fatal problem for me. Just means I have to study the source structure carefully, at length, before I can know where to make changes.

second update:
(2015-05-10) I have now studied the development source code well enough to know all the changes all I need to make (it's "a little more" than what I had supposed at the outset). And I've got it to build from source with Ruby and Node. My next step should be to submit a pull request...

[prather-mcs]

Okay I made a Pull Request:
#851

Here is the general context and the general thinking I used to implement a fix:

• I tried to change the classes that Jasmine uses to be "unique enough" by appending "_jasmine-css" to any class name. So what used to be called .menu should now be called .menu_jasmine-css.
• The only files I made changes were to:
  • /src/html/HtmlReporter.js
  • /src/html/_HTMLReporter.scss
  • /spec/html/HtmlReporterSpec.js
• I made changes to those places only because, as far as I could tell, Jasmine is well-modularized and the only places I needed to change were these three. I definitely had to read code in other files, because ResultsNodes and QueryStrings were included in these files, and I definitely needed to understand them in order not to break them. I also did project-wide / codebase-wide searches for relevant pieces of code.
  One of the many mistakes I may have made was not noticing more code that needed to be changed to implement a fix for this Issue.
• To find each and every place that CSS classes were being created or used in Jasmine, I looked at:
  • every CSS rule in the .scss file
  • every usage of querySelector and querySelectorAll in the codebase
  • every usage of .getAttribute
  • every usage of methods like find that were built on top of querySelector etc...
• This is the list of selectors I found which needed to be changed:
  .alert, .banner, .bar, .description, .duration, .empty, .errored, .exceptions, .failed, .failure-list, .failures, .failures-menu, .menu, .messages, .open, .passed, .payload, .pending, .raise, #raise-exceptions, .result, .result-message, .results, .run-options, .skipped, .spec-detail, .spec-list, .spec-list-menu, .specs, .stack-trace, .suite, .suite-detail, .summary, .symbol-summary, .throw, .throw-failures, #throw-failures, .title, .trigger, .version
  There would be more id name changes to make, except that I had to leave them alone, for reasons I'll write up in another issue. For now, I left most ids alone.

I don't care much if my particular implementation of a fix for this Issue is used. Perhaps someone will judge that there is a better method to get to the same endpoint. I just wrote a fix to get the conversation started. There's got to be more work to do before it's really fixed, right?

---

I've written an explanation here...
https://groups.google.com/forum/#!topic/jasmine-js-dev/IM7wkkJHlIw

...for why I couldn't test my code with rake jasmine either before or after building a new lib from my changes to src and spec