Skip to content

fixed mini browser getSource uri#10481

Merged
msujew merged 2 commits intoeclipse-theia:masterfrom
aditya211935:fix/mini-browser-open-handler-uri
Dec 6, 2021
Merged

fixed mini browser getSource uri#10481
msujew merged 2 commits intoeclipse-theia:masterfrom
aditya211935:fix/mini-browser-open-handler-uri

Conversation

@aditya211935
Copy link
Contributor

@aditya211935 aditya211935 commented Nov 26, 2021

Signed-off-by: Aditya Sharma aditya211935@gmail.com

What it does

Fixes: #10478

Since the URI passed to getSourceUri function doesn't have "http" or "https" scheme, and since a remote URL opened in preview mode doesn't have a a source URI on disk, a simple check is performed to check if the passed URI is equal to MiniBrowserOpenHandler's PREVIEW_URI static variable.

How to test

  1. Press Command + Shift + P to open up command palette. Type "Open URL" and press enter.
  2. Enter any URL that you want to open.
  3. Mini browser will open the specified url in preview mode, and will be docked to the right side panel. Look at its top right side, and an "Open Source" icon will not be visible.

Review checklist

Reminder for reviewers

Signed-off-by: Aditya Sharma <aditya211935@gmail.com>
@aditya211935
Copy link
Contributor Author

aditya211935 commented Nov 26, 2021

@vince-fugnitto I've accepted the terms. I didn't know that you had to login to eclipse to sign ECA.

@aditya211935

This comment has been minimized.

@vince-fugnitto
Copy link
Member

@aditya211935 thank you for signing the eca, everything looks in order 👍

@vince-fugnitto vince-fugnitto added the mini-browser issues related to the mini-browser label Nov 26, 2021
@aditya211935
Copy link
Contributor Author

@vince-fugnitto Any update on when it'll be merged? I can see that all workflow checks have passed.

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aditya211935 Thank you for your contribution. Contributions are merged after at least one approving review has been given (Merging usually happens one or two days after the approval for any other reviewers to object to the merge). See the review guidelines.

I confirmed that the issue exists on master and is addressed by your changes. Very clean solution 👍

  • Opening the mini browser using the Open URL command does not show the Open Source toolbar button.
  • Opening the mini browser by previewing a local html file shows the toolbar button and clicking it opens the original html file correctly.

@aditya211935
Copy link
Contributor Author

@msujew Thanks! 👍

Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for keeping the old checks.

@msujew msujew merged commit afcf799 into eclipse-theia:master Dec 6, 2021
@vince-fugnitto vince-fugnitto added this to the 1.21.0 milestone Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mini-browser issues related to the mini-browser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mini browser preview URL shows open source icon

4 participants