On a page containing the OpenAtrium toolbar Firefox occasionally logs the following warning:

Empty string passed to getElementById().

I traced it back to the usage of data-target="#" in oa-space-nav.tpl.php. This is unnecessary, the toolbar works fine without it.

CommentFileSizeAuthor
#2 2854597-oa_toolbar-jserrorfix-1.patch1.29 KBJorrit

Comments

Jorrit created an issue. See original summary.

Jorrit’s picture

Jorrit commented
StatusFileSize
new 2854597-oa_toolbar-jserrorfix-1.patch1.29 KB
Jorrit’s picture

Jorrit commented
Title: Javascript error due to » Javascript error due to bad Bootstrap HTML
Jorrit’s picture

Jorrit commented
Status: Active » Needs review
mpotter’s picture

mpotter commented

Why would Firefox only "occasionally" report a problem. Seems like either it's a problem or it's not. It would be good to post a procedure for reproducing this problem so we can verify the fix.

Jorrit’s picture

Jorrit commented

Sorry about my confusion wording. The warning is logged on each interaction with a popup powered by Bootstrap like the buttons for spaces in the header.

mpotter’s picture

mpotter commented

I am not able to reproduce this in Firefox 50.1.0 on my Mac. You'll need to post step-by-step instructions for seeing this error. That data-target must have been added for a reason and I don't know if it's something left over from an older version of Bootstrap or if it is still needed for some reason. So without more understanding of this issue and it's underlying cause I can't just remove it.

Jorrit’s picture

Jorrit commented

Is there a publicly accessible demo site with an activated toolbar module?

mpotter’s picture

mpotter commented

You can spin up a free public Atrium site in less than 5 min at getpantheon.com/openatrium

Jorrit’s picture

Jorrit commented

Thanks for the reference to that site. I created a site at http://dev-issue-2854597.pantheonsite.io/test-space. Login with test / testtesttest and go to http://dev-issue-2854597.pantheonsite.io/test-space. Hover the 'Test Space' text in the header. The message 'Empty string passed to getElementById().' will appear.

mpotter’s picture

mpotter commented

Must still be a specific browser issue. I followed your procedure on that test site and using Firefox 50.1.0 on my Mac I did not see any error in the Console when I hover over "Test Space" in the header.

dpoletto’s picture

dpoletto commented

Tried hovering the "Test Space" text in the header with Mozilla Firefox 51.0.1 (64 bit) for Linux and all seems quite normal (no issue so far).

Jorrit’s picture

Jorrit commented

I tried it again and the error appeared after clicking the text, not hovering. I have recorded a small video to demonstrate that it is not made up. If you still can't reproduce it, please close this issue because too much time has been spent on this minor thing already.

https://youtu.be/QF27CL_VVH4

dpoletto’s picture

dpoletto commented

Sorry @Jorrit,

I re-tested now after watching the video you posted and I also noticed (It's a Java Script Warning not an Error!) that every time I hover in and I hover out the "Test Space" link Firefox logs a grandtotal of four (two when I enter, two when I left) consecutive Warnings "Empty string passed to getElementById()" (hovering few times will grow logged lines a lot); that Java Script Warning pops up after others two: "Use of getAttributeNode() is deprecated. Use getAttribute() instead." and "Use of getPreventDefault() is deprecated. Use defaultPrevented instead." those ones both show up when (and each time) the "Test Space" page is loaded.

Definitely not Java Script Errors...just Warnings.

Argus’s picture

Argus commented
Priority: Normal » Minor

Yeah I can also reproduce the Empty string passed to getElementById(). with the latest Firefox on a Mac. This seems to be a known bootstrap-Firefox or jquery issue: Google it. But it doesn't appear in other browsers and doesn't interfere with the use of the site.

To commit the patch we would need to verify if it doesn't introduce problems with other browsers...

Jorrit’s picture

Jorrit commented

The data-target attribute is intended to bind a dropdown button to a specific dropdown. The value must be a valid CSS query, which # is not. data-target is optional. If it is not a valid element, the default behavior is to toggle the open class on the parent element. This behavior is used by the toolbar menu. It does not need the data-target attribute to work.

Argus’s picture

Argus commented

That seems a logical explanation.

mpotter’s picture

mpotter commented
Status: Needs review » Fixed

Tested this a bit and it seems ok. Based on the info above, I committed this to a113a59 in oa_toolbar.

Jorrit’s picture

Jorrit commented

Thanks for reviewing and committing.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.