Closed
Bug 1366214
Opened 8 years ago
Closed 8 years ago
The result not found message should be "Sorry! There are no results in Options for " on Windows
Categories
(Firefox :: Settings UI, defect, P1)
Tracking
()
VERIFIED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox55 | --- | verified |
People
(Reporter: evanxd, Assigned: evanxd)
References
(Blocks 1 open bug)
Details
(Whiteboard: [photon-preference])
Attachments
(1 file)
The result not found message should be "Sorry! There are no results in Options for " on Linux/Windows.
The spec: https://mozilla.invisionapp.com/share/ZDAGPK3AF#/screens/218928235
Flags: qe-verify+
Assignee | ||
Comment 1•8 years ago
|
||
Hi Mike,
Do you know how we load different string keys (or dtd files) for different OS platforms to fix this issue?
Thanks.
Flags: needinfo?(mconley)
Assignee | ||
Comment 2•8 years ago
|
||
I think we could detect OS platform and load different string keys, like
```
let xulRuntime = Components.classes["@mozilla.org/xre/app-info;1"]
.getService(Components.interfaces.nsIXULRuntime);
let message = "";
if (xulRuntime == "Windows" || xulRuntime == "Linux") {
// Load the correct string for Windows and Linux platforms.
message = strings.getFormattedString("searchResults.sorryMessageWin", [query]);
} else {
message = strings.getFormattedString("searchResults.sorryMessage", [query]);
}
```
This approach almost likes [1].
What do you think of the above solution? Is it OK to us?
[1]: http://searchfox.org/mozilla-central/source/browser/components/preferences/in-content/preferences.xul#60-71
Comment 3•8 years ago
|
||
Note that only Windows use Options, while Linux *and* Mac use Preferences.
Not sure if this helps, but we have code around already doing that, e.g. http://searchfox.org/mozilla-central/source/browser/base/content/abouthome/aboutHome.xhtml#67-71
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Summary: The result not found message should be "Sorry! There are no results in Options for " on Linux/Windows → The result not found message should be "Sorry! There are no results in Options for " on Windows
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #3)
> Note that only Windows use Options, while Linux *and* Mac use Preferences.
>
> Not sure if this helps, but we have code around already doing that, e.g.
> http://searchfox.org/mozilla-central/source/browser/base/content/abouthome/
> aboutHome.xhtml#67-71
Hi Francesco,
Thanks for reminding, we only use "Options" on Windows.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → evan
Status: NEW → ASSIGNED
Priority: P2 → P1
Assignee | ||
Updated•8 years ago
|
Attachment #8869892 -
Flags: review?(mconley)
Assignee | ||
Comment 6•8 years ago
|
||
Hi Mike,
Could you help review the patch?
Thanks.
Flags: needinfo?(mconley)
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8869892 [details]
Bug 1366214 - Add a new result not found string for Windows.
https://reviewboard.mozilla.org/r/141056/#review144992
::: browser/locales/en-US/chrome/browser/preferences/preferences.properties:261
(Diff revision 1)
> removeContainerButton2=Don’t remove this Container
>
> # Search Results Pane
> # LOCALIZATION NOTE %S will be replaced by the word being searched
> +searchResults.sorryMessageWin=Sorry! There are no results in Options for “%S”.
> searchResults.sorryMessage2=Sorry! There are no results in Preferences for “%S”.
At this point I would rename this string as searchResults.sorryMessageUnix for clarity.
The current string (with 2) hasn't been pushed out to localizers yet because I'm waiting for the correct strings to land.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8869892 [details]
Bug 1366214 - Add a new result not found string for Windows.
https://reviewboard.mozilla.org/r/141056/#review144992
> At this point I would rename this string as searchResults.sorryMessageUnix for clarity.
>
> The current string (with 2) hasn't been pushed out to localizers yet because I'm waiting for the correct strings to land.
Hi Flod,
Good point. I've updated it.
Assignee | ||
Updated•8 years ago
|
Whiteboard: [photon-preference]
Comment 10•8 years ago
|
||
mozreview-review |
Comment on attachment 8869892 [details]
Bug 1366214 - Add a new result not found string for Windows.
https://reviewboard.mozilla.org/r/141056/#review145998
r=me with the following change
::: browser/components/preferences/in-content/findInPage.js:243
(Diff revision 2)
> let noResultsEl = document.querySelector(".no-results-message");
> noResultsEl.hidden = false;
>
> let strings = this.strings;
> - document.getElementById("sorry-message").textContent =
> - strings.getFormattedString("searchResults.sorryMessage2", [query]);
> +
> + document.getElementById("sorry-message").textContent = Services.appinfo.OS === "WINNT" ?
AppConstants.jsm is loaded already, so please use `AppConstants.platform == "win"` instead since it is more concise.
Attachment #8869892 -
Flags: review+
Comment 11•8 years ago
|
||
mozreview-review |
Comment on attachment 8869892 [details]
Bug 1366214 - Add a new result not found string for Windows.
https://reviewboard.mozilla.org/r/141056/#review146418
r=me with jaws' issue fixed.
Attachment #8869892 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 12•8 years ago
|
||
mozreview-review |
Comment on attachment 8869892 [details]
Bug 1366214 - Add a new result not found string for Windows.
https://reviewboard.mozilla.org/r/141056/#review146868
::: browser/components/preferences/in-content/findInPage.js:243
(Diff revision 2)
> let noResultsEl = document.querySelector(".no-results-message");
> noResultsEl.hidden = false;
>
> let strings = this.strings;
> - document.getElementById("sorry-message").textContent =
> - strings.getFormattedString("searchResults.sorryMessage2", [query]);
> +
> + document.getElementById("sorry-message").textContent = Services.appinfo.OS === "WINNT" ?
Sure. Let's do it.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•8 years ago
|
||
Updated the patch for review comments. Let's land it after the try[1] is good.
[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=51b6f43650e3
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 15•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3a1143c269e5
Add a new result not found string for Windows. r=jaws,mconley
Keywords: checkin-needed
Comment 16•8 years ago
|
||
bugherder |
Comment 17•8 years ago
|
||
Build ID: 20170529030204
User Agent:Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
Verified as fixed on Firefox Nightly 55.0a1 on Windows 10 x 64 and Windows 7 x32.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•