Closed
Bug 1448585
Opened 8 years ago
Closed 8 years ago
Avoid useless use of strlen for DEFAULT_HOME_PAGE
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla61
| Tracking | Status | |
|---|---|---|
| firefox61 | --- | fixed |
People
(Reporter: hsivonen, Assigned: hsivonen)
References
Details
Attachments
(1 file)
Let's use AssignLiteral for a literal.
| Comment hidden (mozreview-request) |
Comment 2•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8962080 [details]
Bug 1448585 - Use NS_LITERAL_STRING for DEFAULT_HOME_PAGE. .
https://reviewboard.mozilla.org/r/230924/#review236760
::: dom/base/nsGlobalWindowOuter.cpp:4986
(Diff revision 1)
> if (homeURL.IsEmpty()) {
> // if all else fails, use this
> #ifdef DEBUG_seth
> printf("all else failed. using %s as the home page\n", DEFAULT_HOME_PAGE);
> #endif
> - CopyASCIItoUTF16(DEFAULT_HOME_PAGE, homeURL);
> + homeURL.AssignLiteral(DEFAULT_HOME_PAGE);
Let's do `homeURL = NS_LITERAL_STRING(DEFAULT_HOME_PAGE);` as that way we get to do no runtime character width changes at all.
Attachment #8962080 -
Flags: review?(nika) → review+
| Comment hidden (mozreview-request) |
| Assignee | ||
Comment 4•8 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8962080 [details]
Bug 1448585 - Use NS_LITERAL_STRING for DEFAULT_HOME_PAGE. .
https://reviewboard.mozilla.org/r/230924/#review236760
> Let's do `homeURL = NS_LITERAL_STRING(DEFAULT_HOME_PAGE);` as that way we get to do no runtime character width changes at all.
Fixed. Thanks. I wonder if we should
1. Tell people to use `u""` literals with nsAString
2. Have a lint to block new calls to the `char` version of `AssignLiteral` on `nsAString`
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6580b15bcd33
Use NS_LITERAL_STRING for DEFAULT_HOME_PAGE. r=mystor.
Comment 6•8 years ago
|
||
(In reply to Henri Sivonen (:hsivonen) from comment #4)
> 2. Have a lint to block new calls to the `char` version of `AssignLiteral`
> on `nsAString`
How about `= delete`ing the `char` overload? It would be better to find the mistake earlier than lint.
| Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #6)
> (In reply to Henri Sivonen (:hsivonen) from comment #4)
> > 2. Have a lint to block new calls to the `char` version of `AssignLiteral`
> > on `nsAString`
>
> How about `= delete`ing the `char` overload? It would be better to find the
> mistake earlier than lint.
We could do that after changing the existing callers.
Comment 8•8 years ago
|
||
| bugherder | ||
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Updated•7 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•