RFR [15] 8237909: Remove zipped index files feature

[Hannes Wallnöfer]

Am 28.01.2020 um 19:41 schrieb Pavel Rappo <pavel.rappo at oracle.com>:
> 
> Hannes,
> 
>> On 28 Jan 2020, at 17:15, Hannes Wallnöfer <hannes.wallnoefer at oracle.com> wrote:
>> 
>> Hi Pavel, 
>> 
>> I fully concur with your reasoning for removing the jszip feature.
> 
> Removing JSZip is incidental here. It's the whole "zipped index files" feature
> that is being removed.
> 

Understood.

>> In script.js you left in the checks whether the *SearchIndex variables are undefined:
>> 
>>     if (!moduleSearchIndex) {
>>         ...
>>     }
>> 
>> These if-statements can be removed as the conditions will always be true (the vars are declared a few lines above but not assigned any value, so they will always be undefined). It’s ok to just invoke createElem(…) for all scripts/index files.
> 
> I will remove those conditional checks, thanks.

Actually my analysis was not correct since the variables are defined in top-level script code while the script tag creation happens in function code, so other things could happen between execution of the two. The conclusion is still correct though since the *-search-index.js files is the only code that assign to these variables.

> 
>> I also wonder if there is any benefit left in creating the script tags dynamically vs. just adding them to the pages statically. I’m fine with keeping it that way for now, but maybe we could file another issue to check if there’s any reason to keep this over static <script> tags.
>> Everything else looks good to me.
>> Hannes
> 
> One benefit of creating those script elements dynamically seems to be the effectively
> asynchronous transfer of the .js index files. Those files should be transferred
> asynchronously, no question about that. We should make this explicit, though.
> 
> I'm not a web developer, but from what I've read there is more than one way to
> load a script asynchronously. I wanted to muse over all the options properly in
> the context of 8236935: Improve UX of the search control, where this can be
> evaluated together with the different sorts of script loading events.
> 

You’re right, async loading is probably the main benefit of loading scripts this way. It seems HTML5 provides an `async` attribute to the script tag for this purpose, and there’s also the older `defer` attribute for older browsers. I think I’ll file a separate issue for this as the implementation detail of async loading doesn’t really belong to a UX bug (unless we decide to switch to synchronous loading, which I don’t think we should).

> 
> I intend to let this RFR sit here for at least 2 weeks. This, hopefully, will
> make it accessible to more pairs of eyes. Maybe the original authors will chime in.
> The perceived severity of this change should not be underestimated.
> 

Sounds like a good plan.

Hannes

> -Pavel
>