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

Handling of old/poorly formed Javascript window.location = url #31

Closed
DruSatori opened this issue Dec 7, 2016 · 4 comments
Closed

Handling of old/poorly formed Javascript window.location = url #31

DruSatori opened this issue Dec 7, 2016 · 4 comments

Comments

@DruSatori
Copy link

DruSatori commented Dec 7, 2016

Right now, there is an issue in the handling of the window.location with regards to handling the old and incorrect, but still commonly used javascript syntax of setting the window.location to a url instead of referencing it as window.location.href = url.

Non-Working:

window.location = "/";

Working:

window.location.href` = "/"

Since I am unaware of a way to have a property return one datatype but accept another in the setter, n theory, using the object type as the property type the code in Window.cs could be modified from:

        /// <summary>
        /// Gets the location of the currently contained document.
        /// </summary>
        public ILocation Location
        {
            get { return Document.Location; }
        }

to the less elegant, but functional:

         /// <summary>
        /// Gets the location of the currently contained document.
        /// </summary>
        public object Location
        {
            get { return Document.Location; }
	    set
		{
			String locationAsUrl = value.ToString();
			// probably need to validate the resulting string
			if (validateUrl(locationAsUrl))
			{
				Document.Location.Href = locationAsUrl;
			}
		}
        }

A big weakness here is that it would require typecasting the returned object to an ILocation for any strongly typed code not using the var notation.

Your thoughts ?

@FlorianRappl
Copy link
Contributor

FlorianRappl commented Dec 7, 2016

The proposed way negatively impacts the core AngleSharp - just because of one quirk in a scripting language. Usually, AngleSharp handles such things via DOM attributes. Apparently, this was once the way to do it (or at least acceptable), so there should be an attribute similar to DomPutForwardsAttribute (or another option to this attribute).

I will think about the best solution in this case. An enhancement should be available with the next release.

Remark: Please work on your formatting here on GitHub. Code blocks are written with 3 backticks and no indentation (plus optionally opened with the used language, e.g., cs). Otherwise, it is quite difficult to read.

@DruSatori
Copy link
Author

I suspected as much on the change, that's why I posted this. I was looking for alternatives in the exiting core, but don't see any other similar implementations as of yet.

On the formatting, it is now fixed. Interestingly the 'code' button in the editor toolbar only puts in two marks, not three, and if you have the code selected when you press it, it wraps the code in single marks. The original post has been modified accordingly.

@FlorianRappl
Copy link
Contributor

Again, these are attributes. The [DomPutForwards] annotation is used a couple of times; and consumed accordingly from the scripting extension.

FlorianRappl added a commit that referenced this issue May 13, 2019
@FlorianRappl
Copy link
Contributor

Landed in devel.

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

2 participants