Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] New metablock @filetype #1842

Closed
sjehuda opened this issue Jun 29, 2023 · 17 comments
Closed

[Feature] New metablock @filetype #1842

sjehuda opened this issue Jun 29, 2023 · 17 comments

Comments

@sjehuda
Copy link

sjehuda commented Jun 29, 2023

Motivation

Preface

I have wrote a userscript that converts structured data files (i.e. XML and JSON) into human readable HTML files.

Overhaul, the program works as expected; documents are processed and forged successfully into HTML.

All the extra functionalities work well when document.contentType (read only) does not contain xml.

However, when document.contentType has xml, any code that contains querySelector would fail because the document is believed to be whatever document.contentType has determined upon initial request (an XML file), and, apparently, the web browser doesn't check document once it has been loaded.

An error message from the program suggests the file processed is XML when in reality it is an HTML, hence hinders other functions to be executed:

Expected Behavior

Execute actions on document as as HTML.

Actual Behavior

Browser blocks HTML actions (throws errors) because it is believed to be an XML.

Proposed Solution

This is a request to add new API to GM.xmlHttpRequest, or to the core of the Greasemonkey extension, that can influence a change in document.contentType.

See #1842 (comment)

Use Cases

Script

https://greasyfork.org/en/scripts/465932-newspaper-native-rss-reader

Test Page

https://lzone.de/liferea/blog/feed.xml application/xml

Note

More information at greasemonkey/greasemonkey#3164

@tophf
Copy link
Member

tophf commented Jun 29, 2023

I don't think the ability to change headers is within the scope of a userscript manager. @gera2ld?

@sjehuda
Copy link
Author

sjehuda commented Jun 29, 2023 via email

@tophf
Copy link
Member

tophf commented Jun 29, 2023

Judging by the description GM.xmlHttpRequest is entirely unrelated. You seem to want to change the content type of the web page. I don't see it as something a userscript manager should do.

@sjehuda
Copy link
Author

sjehuda commented Jun 29, 2023 via email

@tophf
Copy link
Member

tophf commented Jun 29, 2023

Userscript managers shouldn't change the type of the document, it's beyond their scope.

@sjehuda
Copy link
Author

sjehuda commented Jun 29, 2023 via email

@sjehuda sjehuda changed the title [Feature] equivalent to webRequest.onHeadersReceived [Feature] New metablock @filetype Jun 29, 2023
@tophf
Copy link
Member

tophf commented Jun 29, 2023

The deciding factor is common sense. Userscript manager's purpose is to inject and manage userscripts. This is the vision we implement. We regularly reject the temptation to add a feature that would be useful only to a couple of userscripts.

@leumasme
Copy link

leumasme commented Jun 29, 2023

This is the actual error:

DOMException: Failed to execute 'insertAdjacentHTML' on 'Element': The provided markup is invalid XML, and therefore cannot be inserted into an XML document.

Caused by

function settingsPage() {
  document.body.insertAdjacentHTML('beforeend', htmlSettings); // <-- Error!
  let btn = document.querySelector('#about-settings');
  let dia = document.querySelector('#page-settings');
  btn.addEventListener ("click", function() {
    if (document.contentType.endsWith('xml')) {
      alert('Setting are disabled on this page.');
      return;
    }
    dia.style.display = 'block';
  });
}

The value of htmlSettings being:

<div class="about-newspaper" id="page-settings">
  <h1>Settings</h1>
  <p>Settings are saved instantly upon click.</p>
  <table>
    <!-- tr>
      <td>
        <span>Articles</span>
      </td>
      <td>
        <span><input type="number" name="item-number" id="item-number" min="2" max="50"/> <label>№</label></span>
        <p>Maximum number of articles to display (2 - 50)</p>
      </td>
    </tr -->
    <tr>
      <td>
        <span>Text Size</span>
      </td>
      <td>
        <span>
          <input type="number" name="font-size" id="font-size" min="15" max="50"/>
          <label>px.</label>
        </span>
      </td>
    </tr>
    <tr>
      <td>
        <span>Font Style</span>
      </td>
      <td>
        <span>
          <select name="font-type" id="font-type">
            <option value="arial">Arial</option>
            <option value="sans">Sans</option>
            <option value="serif">Serif</option>
            <option value="system-ui">System</option>
          <select>
        </span>
      </td>
    </tr>
    <tr>
      <td>
        <span for="view-mode">Mode</span>
      </td>
      <td>
        <span>
          <select name="view-mode" id="view-mode">
            <option value="bright">Bright</option>
            <option value="dark">Dark</option>
            <!-- option value="sepia">Sepia</option -->
          <select>
        </span>
      </td>
    </tr>
    <!-- tr>
      <td>
        <span>Podcast</span>
      </td>
      <td>
        <span>
          <input type="checkbox" name="play-enclosure" id="play-enclosure"/>
          <label for="play-enclosure">Play Audio</label>
        </span>
        <span>
          <input type="checkbox" name="disable-enclosure" id="disable-enclosure"/>
          <label for="disable-enclosure">Disable Enclosures</label>
        </span>
      </td>
    </tr>
    <tr>
      <td>
        <span>Hide</span>
      </td>
      <td>
        <span>
          <input type="checkbox" name="hide-audio" id="hide-audio"/>
          <label for="hide-audio">Audio</label>
        </span>
        <span>
          <input type="checkbox" name="hide-image" id="hide-image"/>
          <label for="hide-image">Image</label>
        </span>
        <span>
          <input type="checkbox" name="hide-media" id="hide-media"/>
          <label for="hide-media">Media</label>
        </span>
        <span>
          <input type="checkbox" name="hide-video" id="hide-video"/>
          <label for="hide-video">Video</label>
        </span>
      </td>
    </tr -->
  </table>
  <p>Click reload to apply changes.</p>
  <button id="reload">Reload</button>
  <button id="close">Close</button>
</div>

If we run this through an XML validator to see why this isn't valid XML, we get this:

37:11	The element type "select" must be terminated by the matching end-tag "</select>".

Which corresponds to this part of your html:

30 | <span>
31 | <select name="font-type" id="font-type">
32 | <option value="arial">Arial</option>
33 | <option value="sans">Sans</option>
34 | <option value="serif">Serif</option>
35 | <option value="system-ui">System</option>
36 | <select> <!-- ERROR HERE -->
37 | </span>
38 | </td>
39 | </tr>
40 | <tr>
41 | <td>
42 | <span for="view-mode">Mode</span>

There is indeed an error in your settings page HTML. The only reason this works in regular HTML is because browsers will try to correct mistakes in html, which includes missing closing tags. The browser here sees the </span>, notices that the current tag is <select> and not <span> and injects two closing select tags before the closing span.
This doesn't work in an XML document as the browser is strict there.

If we correct this error (and another identical one in line 51), the error disappears.
When then also patching out your xml detection to make dark mode and settings available, both seem to work fine.
I believe this should fully solve your issue - no need to change the type of the document (which most likely would absolutely not work anyway).

@leumasme
Copy link

leumasme commented Jun 29, 2023

While I'm already on the task of fixing your script errors:

Greasemonkey API Not Available
Newspaper (Native RSS Reader).user.js:4885 TypeError: Cannot read properties of null (reading 'setAttribute')

This error can come not just from the Greasemonkey api being unavailable, but from the script settings not being populated - You loop through your input/select elements

    for (const controller of document.querySelectorAll('#page-settings input, #page-settings select')) {

which are

input#font-size
select#font-type
select#view-mode

and then on each call GM.getValue(controller.name);
This will result in undefined if the setting was never changed.
The first setting control is font-size so we enter

case ('number'):
          controller.setAttribute('value', value);

which then calls setAttribute on undefined. Since you wrapped way too much code in a try-catch, this still results in a "Greasemonkey API Not Available" error.
You should set the settings values to defaults if they aren't set, or fall back to default values when getting them if they are undefined.

@leumasme
Copy link

leumasme commented Jun 29, 2023

Some other issues actually are caused by the document being XML, such as element.style not being defined by default since you're technically dealing with XML elements.
You seem to have already found a workaround for this - creating a html document and inserting it is a good idea. That way you can work with regular HTML instead of dealing with the XML oddities without requiring the entire site to be pretend-loaded as HTML.

function createPage() {
  let domParser = new DOMParser();
  let newDocument =
    domParser
    .parseFromString('', 'text/html');
  return newDocument;
}
function renderActivityStream(jsonFile) {

  let newDocument = createPage();

  newDocument.title = jsonFile.title;
  if (jsonFile.language) {
    newDocument
    .documentElement
    .setAttribute('lang', jsonFile.language);
  }

  let feed = newDocument.createElement('div');
  feed.id = 'feed';

however you then also need to use newDocument consistently - if you instead use document.createElement, it will create an XML object.
That is why the following is true:

document.querySelector("#links-bar").style === undefined
document.querySelector("#title").style instanceof CSSStyleDeclaration

since the different elements are created in different ways

function linksBar() {
  let divElement = document.createElement('div'); // document -> this creates an XML object
  divElement.innerHTML = htmlBar;
  let title = newDocument.createElement('h1'); // newDocument -> this creates a HTML element
  if (jsonFile.title) {
    title.textContent = jsonFile.title;
  } else {
    title.textContent = document.location.hostname;
  }
  title.id = 'title';
  feed.append(title);

and style is a property of HTML elements that isn't present on XML elements by default

why did i spend my time on this, i only wanted to check if @\run-at context-menu support was planned
anyway, i think this issue can be closed

@sjehuda
Copy link
Author

sjehuda commented Jun 29, 2023

This is the actual error:

The Invalid XML error was fixed.

I'm uploading a new version now.

@sjehuda
Copy link
Author

sjehuda commented Jun 29, 2023

You should set the settings values to defaults if they aren't set, or fall back to default values when getting them if they are undefined.

I did this so it would be easier to execute the script from Inspector in web browsers without Userscript enabled.

I'd be glad to get some help on this.

I'm not familiar with GM settings much.

@sjehuda
Copy link
Author

sjehuda commented Jun 29, 2023

You seem to have already found a workaround for this - creating a html document and inserting it is a good idea. That way you can work with regular HTML instead of dealing with the XML oddities without requiring the entire site to be pretend-loaded as HTML.

Yes, but only as long as the document is not replaced (yet) by newDocument.

Funny to mention that the output of newDocument.contentType is always HTML, even when document is XML, but when document is XML, then - as I mentioned - after document is replaced by newDocument then it's still XML.

@sjehuda
Copy link
Author

sjehuda commented Jun 29, 2023

I have intentionally made the functions called from extensionLoader() to deal with document, because otherwise, I would have to pass newDocument and return newDocument which seems weird and is not modular-wise.

since the different elements are created in different ways

function linksBar() {
  let divElement = document.createElement('div'); // document -> this creates an XML object
  divElement.innerHTML = htmlBar;
  let title = newDocument.createElement('h1'); // newDocument -> this creates a HTML element
  if (jsonFile.title) {
    title.textContent = jsonFile.title;
  } else {
    title.textContent = document.location.hostname;
  }
  title.id = 'title';
  feed.append(title);

and style is a property of HTML elements that isn't present on XML elements by default

Nice. I didn't know that.

Please look into function placeNewDocument()
I have placed some functions there instead of function extensionLoader() just so I would be able to process crucial elements while the document is still HTML.

  newDocument = purgeStylesheets(newDocument); // FIXME
  newDocument = setCssStylesheet(newDocument); // FIXME TODO SET XML-STYLESHEET IF CONTENT-TYPE IS XML
  if (xmlStylesheet) {
    stylesheetMessage(newDocument)
  }

@sjehuda
Copy link
Author

sjehuda commented Jun 29, 2023

Video preview of the script in action in a page that document.contentType doesn't contain xml.

simplescreenrecorder-2023-06-29_21.12.45.mp4

@tophf tophf closed this as not planned Won't fix, can't repro, duplicate, stale Jul 2, 2023
@sjehuda
Copy link
Author

sjehuda commented Jul 3, 2023 via email

@sjehuda
Copy link
Author

sjehuda commented Apr 8, 2024

I have managed to overcome this setback, namely by passing the HTML element newDocument and processing it in all ways feasible (see preProcess) and then replacing document by newDocument. See v24.04.06 vs. v24.04.08.

There was also another matter which required to create an element and change CSS Stylesheet in order to get the expected behaviour, because JavaScript attribute style does not work with document.contentType that ends with XML. See v24.04.08 vs. v24.04.09.

The only functionality which remains not to work upon XML is mode switcher (bright and dark) due to attribute style, yet it might be possible to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants