Open Bug 1529127 Opened 5 years ago Updated 2 years ago

Use URL for the filename of internet shortcuts without page titles on Windows (remove bogus attempt to get `noPageTitle` from pageinfo.properties from widget/windows/nsDataObj.cpp )

Categories

(Core :: Widget: Win32, enhancement, P3)

Unspecified
Windows
enhancement

Tracking

()

People

(Reporter: nautilus, Unassigned)

References

Details

(Whiteboard: widget-next)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

The pageInfo.properties file is going to be mostly migrated to Fluent by this patch: https://bugzilla.mozilla.org/show_bug.cgi?id=1517493

All strings except for the string

"noPageTitle=Untitled Page:"

which is referenced in

widget/windows/nsDataObj.cpp

will be migrated. This last string should also be migrated so that pageInfo.properties can be deleted.

This string wasn't migrated because it was beyond the scope of the original bug.

Status: UNCONFIRMED → NEW
Component: Untriaged → Page Info Window
Depends on: 1517493
Ever confirmed: true
Version: 65 Branch → Trunk

Per Florian's comment in https://phabricator.services.mozilla.com/D16931#553078 this reference is actually broken. We should take a step back and consider what to do here given that, apparently, nobody has noticed this in years (maybe ever, since the code started using that string). Perhaps it's just dead code?

Summary: Last string in pageInfo.properties should be migrated to Fluent so it can be deleted → Figure out what to do with attempts to reference `noPageTitle` from widget/windows/nsDataObj.cpp that don't work
Component: Page Info Window → Widget: Win32
Product: Firefox → Core

(In reply to :Gijs (he/him) from comment #1)

Perhaps it's just dead code?

Looks like it isn't. To see this, on Windows (and only on Windows?!), go to e.g. https://www.kernel.org/doc/Documentation/vm/transhuge.txt (which has no <title>), and drag the identity block (lock icon / (i) icon) to your desktop.

We basically attempt to find the page title, and then try to get a localized string for "this has no title", and (because that also fails, also before the pageinfo changes), fall back to "Untitled" as the filename, here: https://searchfox.org/mozilla-central/rev/4587d146681b16ff9878a6fdcba53b04f76abe1d/widget/windows/nsDataObj.cpp#1100 and here: https://searchfox.org/mozilla-central/rev/4587d146681b16ff9878a6fdcba53b04f76abe1d/widget/windows/nsDataObj.cpp#1057 .

That might be fine for English speakers, but is clearly suboptimal everywhere else.

On mac, we use the page URL as the filename ( https://searchfox.org/mozilla-central/rev/4587d146681b16ff9878a6fdcba53b04f76abe1d/widget/cocoa/nsDragService.mm#218-222 ). That seems like a good solution that also doesn't require localization.

Of course, things are never that simple - some of the characters that you'd normally have in URLs (esp. '/', ':') are not valid in filenames on Windows NTFS and/or FAT filesystems. I think one typical solution to that is to replace those with underscores. I'm unsure if we need to do that ourselves or if Windows can do it for us.

Masayuki-san, would that be a reasonable solution for the windows case? Also, could we somehow fix the duplication here, e.g. could we drop the ansi variants and just return unicode data, or would that break some consumers?

Flags: needinfo?(masayuki)
Summary: Figure out what to do with attempts to reference `noPageTitle` from widget/windows/nsDataObj.cpp that don't work → Figure out what to do with attempts to reference `noPageTitle` in pageinfo.properties from widget/windows/nsDataObj.cpp that don't work

Google Chrome uses URL with replacing unusable characters with -. So, I think that it's reasonable approach.

For the latter, I'm not sure honestly. However, dropped applications may request ANSI data according to IDataObject::GetData()'s spec. So, I think that we should keep it for compatibility with non-Unicode applications.

Flags: needinfo?(masayuki)
OS: Unspecified → Windows
Summary: Figure out what to do with attempts to reference `noPageTitle` in pageinfo.properties from widget/windows/nsDataObj.cpp that don't work → Use URL for the filename of internet shortcuts without page titles on Windows (remove bogus attempt to get `noPageTitle` from pageinfo.properties from widget/windows/nsDataObj.cpp )
Priority: -- → P3
Whiteboard: widget-next
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.