Bug 207749 - [Win][MiniBrowser] Add 'Go Home' menu item and toolbar button
Summary: [Win][MiniBrowser] Add 'Go Home' menu item and toolbar button
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-14 00:05 PST by Fujii Hironori
Modified: 2020-02-19 18:11 PST (History)
6 users (show)

See Also:


Attachments
Patch (10.72 KB, patch)
2020-02-14 00:08 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (9.18 KB, patch)
2020-02-18 22:24 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2020-02-14 00:05:26 PST
[Win][MiniBrowser] Add 'Go Home' menu item and toolbar button
Comment 1 Fujii Hironori 2020-02-14 00:08:43 PST
Created attachment 390733 [details]
Patch
Comment 2 Ross Kirsling 2020-02-18 13:20:38 PST
Comment on attachment 390733 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=390733&action=review

> Tools/MiniBrowser/win/MainWindow.cpp:414
> +void MainWindow::goHome()
> +{
> +    if (s_commandLineOptions.requestedURL.length())
> +        loadURL(s_commandLineOptions.requestedURL);
> +    else
> +        loadURL(L"https://www.webkit.org/");
> +}

Is there any precedent for using the requested initial URL as a home URL?
Comment 3 Fujii Hironori 2020-02-18 18:39:18 PST
Comment on attachment 390733 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=390733&action=review

>> Tools/MiniBrowser/win/MainWindow.cpp:414
>> +}
> 
> Is there any precedent for using the requested initial URL as a home URL?

Good point.
There is no precedent. GTK MiniBrowser is using "http://www.webkitgtk.org/" as the fixed home page URL.
But, I want to customize home page URL.
Adding a command line option doesn't look good.

  MiniBrowser.exe --homepage <homePageURL>

I'm going to add a registry key for home page URL.
Comment 4 Fujii Hironori 2020-02-18 19:18:27 PST
Mac MiniBrowser has "Set Default URL to Current URL" menu item. (Bug 146780)
This seems the best.
Comment 5 Fujii Hironori 2020-02-18 22:24:38 PST
Created attachment 391143 [details]
Patch
Comment 6 Ross Kirsling 2020-02-19 10:08:41 PST
Comment on attachment 391143 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391143&action=review

> Tools/MiniBrowser/win/MainWindow.cpp:-594
> -    strPtr[INTERNET_MAX_URL_LENGTH - 1] = 0;

I take it this wasn't actually necessary?
Comment 7 Fujii Hironori 2020-02-19 17:59:35 PST
Comment on attachment 391143 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391143&action=review

Thank you for the review.

>> Tools/MiniBrowser/win/MainWindow.cpp:-594
>> -    strPtr[INTERNET_MAX_URL_LENGTH - 1] = 0;
> 
> I take it this wasn't actually necessary?

Yes, I tested.
Comment 8 Fujii Hironori 2020-02-19 18:10:47 PST
Comment on attachment 391143 [details]
Patch

Clearing flags on attachment: 391143

Committed r257024: <https://trac.webkit.org/changeset/257024>
Comment 9 Fujii Hironori 2020-02-19 18:10:50 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2020-02-19 18:11:16 PST
<rdar://problem/59614387>