CSS fallback script checks the wrong element in LinkTagHelper

applejag commented 3 years ago

Describe the bug

The <script> tag generated by LinkTagHelper (as seen here in the LinkTagHelper class and here in the LinkTagHelper_FallbackJavaScript.js) references the last script tag on the page, and then takes the .previousElementSibling of that HTML element. If I have any other script tags on the page, this fallback script will fail and will always load in the fallback source.

To Reproduce

Will be using PaperCSS as example here https://www.getpapercss.com/, hosted via UNPKG.

Create ASP.NET MVC project, ex:
```
dotnet new mvc -n LinkTagHelperIssue
```
Create directory for fallback file at wwwroot/lib/papercss/dist, ex:
```
mkdir -pv LinkTagHelperIssue/wwwroot/lib/papercss/dist
```
Download https://unpkg.com/papercss@1.8.1/dist/paper.min.css and place the downloaded file at wwwroot/lib/papercss/dist/paper.min.css, ex:
```
wget 'https://unpkg.com/papercss@1.8.1/dist/paper.min.css' -O LinkTagHelperIssue/wwwroot/lib/papercss/dist/paper.min.css
```

Edit Views/Shared/_Layout.cshtml, add the following to the end of the <head> tag:

<link rel="stylesheet"
   href="https://unpkg.com/papercss@1.8.1/dist/paper.min.css"
   integrity="sha384-R9lNp8nWuBGkBw8HAeT4RYv8Fmj1mD45av7OFm17UfasvI47Yuk/Sy8wUO5p/184"
   crossorigin="anonymous"
   asp-fallback-href="~/lib/papercss/dist/paper.min.css"
   asp-fallback-test-class="paper" asp-fallback-test-property="border" asp-fallback-test-value="1px solid #c1c0bd" />

Run the project, ex:
```
dotnet run -p LinkTagHelperIssue
```
Open the page in web browser, for example Chrome. Open the inspector tool and look inside the <head> tag, and see that there are two <link> tags referencing paper.min.css, but one from unpkg.com and one from the same site:

Further technical details

ASP.NET Core version: 5.0.0 (have NOT tested on any other version though)
Ran this from dotnet CLI on both Windows and Ubuntu, and from VS on Windows
VS: Microsoft Visual Studio Community 2019, Version 16.8.2

dotnet --info on WSL instance I've tried this on

``` ❯ dotnet --info .NET SDK (reflecting any global.json): Version: 5.0.100 Commit: 5044b93829 Runtime Environment: OS Name: ubuntu OS Version: 20.10 OS Platform: Linux RID: ubuntu.20.10-x64 Base Path: /usr/share/dotnet/sdk/5.0.100/ Host (useful for support): Version: 5.0.0 Commit: cf258a14b7 .NET SDKs installed: 3.1.404 [/usr/share/dotnet/sdk] 5.0.100 [/usr/share/dotnet/sdk] .NET runtimes installed: Microsoft.AspNetCore.App 3.1.10 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.0 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.NETCore.App 3.1.10 [/usr/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.0 [/usr/share/dotnet/shared/Microsoft.NETCore.App] ```

dotnet --info from Windows instance I've tried this on

``` .NET SDK (reflecting any global.json): Version: 5.0.100 Commit: 5044b93829 Runtime Environment: OS Name: Windows OS Version: 10.0.19041 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\5.0.100\ Host (useful for support): Version: 5.0.0 Commit: cf258a14b7 .NET SDKs installed: 3.0.101 [C:\Program Files\dotnet\sdk] 3.1.100 [C:\Program Files\dotnet\sdk] 3.1.302 [C:\Program Files\dotnet\sdk] 5.0.100 [C:\Program Files\dotnet\sdk] .NET runtimes installed: Microsoft.AspNetCore.All 2.1.23 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.1.23 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.0.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 2.1.23 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.0.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.0.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] ```

applejag commented 3 years ago

Don't see why this solution does not use getElementById on some generated value that would be applied to the <meta> tag.

So the generated <meta> tag becomes something like:

<meta id="_102faiw9dk21awd" name="x-stylesheet-fallback-test" content="" class="paper">

And then in the js, change it with something like:

-  function loadFallbackStylesheet(cssTestPropertyName, cssTestPropertyValue, fallbackHrefs, extraAttributes) {
+  function loadFallbackStylesheet(cssTestPropertyName, cssTestPropertyValue, fallbackHrefs, extraAttributes, metaTagId) {
      var doc = document,
-         // Find the last script tag on the page which will be this one, as JS executes as it loads
-         scriptElements = doc.getElementsByTagName("SCRIPT"),
-         // Find the meta tag before this script tag, that's the element we're going to test the CSS property on
-         meta = scriptElements[scriptElements.length - 1].previousElementSibling,
+         // Find the meta tag that has the test CSS class on it
+         meta = doc.getElementById(metaTagId),
          // Get the current style of the meta tag starting with standards-based API and falling back to <=IE8 API
          metaStyle = (doc.defaultView && doc.defaultView.getComputedStyle) ? doc.defaultView.getComputedStyle(meta)
              : meta.currentStyle,
          i;

          //snip

Actually, mind if I just add a PR on this matter?

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

lonix1 commented 2 years ago

@jilleJr Do you still observe this problem with v6?

applejag commented 2 years ago

@jilleJr Do you still observe this problem with v6?

Haven't tested this in a long while. Actually haven't worked with .NET at all in over a year.

dotnet / aspnetcore