Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact mboldan@mozilla.com Reporter
	Description • 6 years ago

[Affected versions]:
 Windows 10 x64
 Ubuntu 14.04 LTS x64
 mac OS X 10.12.6

[Affected platforms]:
 Fx 59.0b11
 Fx 60.0a1

[Steps to reproduce]:
 1. Launch Firefox.
 2. Go to: http://bsmedberg.github.io/firefox-regression-range-finder/# .
 3. Enter a last good build ID and first bad build ID, then click on "Generate Link".

[Expected result]:
 The website generates a pushlog.

[Actual result]:
 The "Generate Link" button is not functional.

[Regression range]:
 Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=38614ffd21d195add38b50056b89d5f99b53baee&tochange=ce14f861f1a8139c7e94b7aa171ec6b0d3808cbf

	Daniel Holbert [:dholbert]
	Comment 1 • 6 years ago

Thanks for the report! Though, just a heads-up:  "Layout: Form Controls" is typically about *layout* issues with form controls (i.e. the controls not *rendering* properly).  In this case, the reported issue seems to be that the page doesn't do anything when you click the button -- and that typically indicates a JS/DOM issue, with whatever script the button is supposed to kick off.  So, this is in the wrong component right now.

Anyway -- in this case, when I click the button, I see this in my error console:
> ReferenceError: generateLink is not defined  bsmedberg.github.io:1:1


It looks like that function lives in a JS file, which this page imports like so:
>   <script src="regression-range.js" type="application/x-javascript;version=1.8"></script>

Given that and given your reported regression range, I'd bet this is a regression from bug 1428745 ("Remove support for version parameter from script loader").  Baku, could you take a look?

	Daniel Holbert [:dholbert]
	Comment 2 • 6 years ago

Yeah, looks like that bug mentions having added the following note to MDN:
> Note:  in Firefox you could specify the version of JavaScript contained in a <script> element by including a non-standard version parameter inside the type attribute — for example  type="application/javascript;version=1.8". This has been removed in Firefox 59 (see bug 1428745).
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script

So this was definitely broken as a result of that bug (intentionally, due to low usage of this feature, it seems).

So this is probably a case where the site needs a fix (and it looks like it's forkable).  Maybe baku knows what the correct site-fix is?  (I'm not sure if the site is depending on that 'type' attribute to enable certain features/etc...)  It looks like the site does have a "View/fork me on github" link, so we can presumably suggest changes that way, and I imagine bsmedberg will gladly merge them. :)

	Daniel Holbert [:dholbert]
	Comment 3 • 6 years ago

[er, I guess this should really be Tech Evang at this point -- reclassifying again]

	Andrea Marchesini [:baku]
	Comment 4 • 6 years ago

> So this is probably a case where the site needs a fix (and it looks like
> it's forkable).  Maybe baku knows what the correct site-fix is? 

The fix is to remove the version param in the type attribute.

This:
<script src="regression-range.js" type="application/x-javascript;version=1.8"></script>

must be:
<script src="regression-range.js" type="application/x-javascript"></script>

Otherwise the script "regression-range.js" is not executed at all.

In bug 1428745 we decided to follow what the spec says and to consider the type attribute value as a mime-type. "application/x-javascript;version=1.8" is not a valid mime-type for JS scripts, so, the whole <script> tag is ignored.

	Daniel Holbert [:dholbert]
	Comment 5 • 6 years ago

Cool, I'll make that change as a pull request on GitHub, and hopefully that'll fix things. Thanks!

	Daniel Holbert [:dholbert]
	Comment 6 • 6 years ago

https://github.com/bsmedberg/firefox-regression-range-finder/pull/1

	Mike Taylor [:miketaylr]
	Comment 7 • 6 years ago

I just pinged the PR to see if we can nudge a merge here. :)

https://github.com/bsmedberg/firefox-regression-range-finder/pull/1/comment#issuecomment-411774646