Extend Browser API to check for custom text location

[amartya4256]

The contribution provides an API to check if the location is the location where custom texts are loaded in a browser. By construct, "about:blank" is used for that purpose. This API can be adapted by the clients instead of explicitly defining the string and checking.

Contributes to #213

[github-actions]

Test Results

   486 files  ± 0     486 suites  ±0   8m 43s ⏱️ + 1m 31s
 4 156 tests + 5   4 148 ✅ + 5   8 💤 ±0  0 ❌ ±0 
16 378 runs  +20  16 286 ✅ +20  92 💤 ±0  0 ❌ ±0 

Results for commit aa4988a. ± Comparison against base commit d200d94.

♻️ This comment has been updated with latest results.

[HeikoKlare]

Seeing the implementation, I am wondering whether it would also be sufficient to just add a showsCustomText() method instead of the isLocationForCustomText(String) one. Then the functionality would be completely independent from the location and the browser handle the state of whether it shows custom text or some URI-based page completely internally and even better hide it from consumers.
@amartya4256 Can you check whether that's possible?

Edit: Please also validate against the WebKit implementation. It seems to already have the about:blank page encoded as kind of a base URI internally. It also has some comments about using about:blank to be consistent with IE, so maybe there is also some further chance to align. I.e., a comment states:

eclipse.platform.swt/bundles/org.eclipse.swt/Eclipse SWT WebKit/cocoa/org/eclipse/swt/browser/WebKit.java

Lines 634 to 637 in 97cf720

	/*
	* If the URI indicates that the page is being rendered from memory
	* (via setText()) then set it to about:blank to be consistent with IE.
	*/

[amartya4256]

Seeing the implementation, I am wondering whether it would also be sufficient to just add a showsCustomText() method instead of the isLocationForCustomText(String) one. Then the functionality would be completely independent from the location and the browser handle the state of whether it shows custom text or some URI-based page completely internally and even better hide it from consumers. @amartya4256 Can you check whether that's possible?

Edit: Please also validate against the WebKit implementation. It seems to already have the about:blank page encoded as kind of a base URI internally. It also has some comments about using about:blank to be consistent with IE, so maybe there is also some further chance to align. I.e., a comment states:

eclipse.platform.swt/bundles/org.eclipse.swt/Eclipse SWT WebKit/cocoa/org/eclipse/swt/browser/WebKit.java

Lines 634 to 637 in 97cf720

	/*
	* If the URI indicates that the page is being rendered from memory
	* (via setText()) then set it to about:blank to be consistent with IE.
	*/

using showsCustomText() won't be possible everywhere since there are Url comparison before the location of the browser has changed. For example in location listener in the changing method which executes before the location is actually changed. Another option was to cache the state type on setText but we don't know where we land with back and forward calls so it cannot be fully implemented. Moreover, I find the current method fine to accept a location as an argument. If you want to check something explictly, that can be passed, otherwise browser.getUrl() can be passed.

The Webkit implementation does make the behaviour consistent by exchanging the places in the API result with about:blank but I think that our implementattion takes thwe technicality away from the client and they dont have to hardcode about:blank anymore and they can leave his decision completely on the browser to tell if it is their custom text page.

[HeikoKlare]

The API extension in general looks good to me. I have some comments on naming, documentation and testing.

Please also adapt the commit message header (and PR name) to be more self-explaining. I.e., instead of "Added API for checking if BASE_URI" use something like "Extend Browser API to check for custom text location"

[HeikoKlare]

Can this be package protected, since the only intended consumer (`Browser´) is in the same package? Please also improve the name according to field name (see comment on the field name for that).

In addition, please check whether this method can be used by the IE and WebKit implementations, which currently have their own ABOUT_BLANK field, which might (at least at some places) be reasonably replaced by this method.

[amartya4256]

Made it package protected but I think refactoring of the browser specific usage can be done in a follow up PR?

[HeikoKlare]

That's also fine. But then it should be done directly after merging this change.

[HeikoKlare]

Thank you for adding the tests! In general, they look fine. My comments are mostly nitpicky style proposals, but some are also on potential race conditions. Anyway, all of them should be easy to resolve.

[HeikoKlare]

Thanks again for the recent update! I have some minor questions/comments left.

[HeikoKlare]

From my side, this is ready to be merged once master branch is open again. Thank you!