twbs / bootstrap

The most popular HTML, CSS, and JavaScript framework for developing responsive, mobile first projects on the web.
https://getbootstrap.com
MIT License
171.01k stars 78.9k forks source link

lack of semi-colons breaks bootstrap js files when tethering #401

Closed cam-intel closed 13 years ago

cam-intel commented 13 years ago

When using bootstrap across a tethered connection at least one js file stops working.

The reason is that many providers (o2, t-mobile, vodafone) across the world embed another js script into web pages. The script is called bmi.js.

This file helps compress many separate js requests into the main page.

The lack of semi-colons in the dropdown js file causes this compression to break, and the dropdown menus stop working on any device if the device is tethered.

The fix is simple: just use semi-colons. The line that breaks is

    clearMenus()
    !isActive &&  ...
fat commented 13 years ago

Are you saying your compressor doesn't add semicolons? that's crazy. You should use a new one! (yui, google closure, uglify.js)

cam-intel commented 13 years ago

Just to be clear: it is not my compressor.

The bmi script is inserted by our evil mobile ISPs - there is nothing one can do to prevent it! (Other than VPN).

So it definitely is worth fixing if you want bootstrap end users to be able to use bootstrap while they are tethered.

fat commented 13 years ago

if your js isn't being compressed, then the following is valid:

  clearMenus()
  !isActive && ....
fat commented 13 years ago

Oh wait... i'm just now understanding... hm... so these devices inject a javascript file which does on the fly "compression" -- and this "compressor" isn't adding semicolons, but is removing new lines?

jmcclell commented 13 years ago

I'm running into the same issue with the default JS min for Java's WRO.

I don't see why semi-colons aren't used to begin with. Your point isn't invalid (that a good compressor should account for this when removing line breaks) but I can't for the life of me see why you would avoid semi-colon usage here.

You're trading the stability of the framework in unknown environments for a couple hundred (if that) bytes of space saved. Is there some other reason semi-colons aren't being used?

mattaustin commented 12 years ago

Stylistically it may be a preference, syntactically it may be perfectly valid, but in practice it causes real-world issues. I would appreciate re-consideration of your semi-colon policy.

tszming commented 12 years ago

I'm just curious why this issue get closed.

Is it solved, or invalid?

fat commented 12 years ago

The reason it was closed is because semicolons were added to the end of lines in 1.4. In 2.0 we removed them again when we introduced the downloader as it safely concats and minifies these files for you. I'm adding semicolons to the end of files in 2.0.1 - which will likely be released tomorrow, to support this mobile oddity.

jmcclell commented 12 years ago

Please don't take them out again. Stylistic preference or not, it would seem the wiser choice is to use them in a library meant for such a broad audience. I'm really glad to see you've reached the same conclusion for this particular issue.

chrisjohnson commented 12 years ago

Add me to the list of people in support of semi-colons. Making assumptions about other tools in use is pretty foolish, it makes more sense to assume that your code is going to run on the lowest common denominator setup.

saimonmoore commented 12 years ago

amen

ShaggyDude commented 12 years ago

Here we go again!

Johann-S commented 5 years ago

our distributed JS files contains semicolons @elimin8r 😉