Bug 207749 – [Win][MiniBrowser] Add 'Go Home' menu item and toolbar button

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Fujii Hironori]

[Win][MiniBrowser] Add 'Go Home' menu item and toolbar button

[Fujii Hironori]

Created attachment 390733 [details]
Patch

[Ross Kirsling]

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?

[Fujii Hironori]

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.

[Fujii Hironori]

Mac MiniBrowser has "Set Default URL to Current URL" menu item. (Bug 146780)
This seems the best.

[Fujii Hironori]

Created attachment 391143 [details]
Patch

[Ross Kirsling]

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?

[Fujii Hironori]

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.

[Fujii Hironori]

Comment on attachment 391143 [details]
Patch

Clearing flags on attachment: 391143

Committed r257024: <https://trac.webkit.org/changeset/257024>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59614387>