Open Bug 460969 Opened 16 years ago Updated 23 days ago

Copy/paste of images from content area lose transparency information

Categories

(Core :: Widget: Win32, defect, P2)

x86
Windows
defect

Tracking

()

Tracking Status
firefox73 --- wontfix
firefox74 --- wontfix

People

(Reporter: jimm, Assigned: ahale, NeedInfo)

References

(Depends on 4 open bugs, Blocks 2 open bugs)

Details

(Whiteboard: widget-next,[win:pasteboard])

Attachments

(4 files, 6 obsolete files)

Outdated bitmap handling code for both rendering and receiving bitmap data from the clipboard causes transparency information to be lost. The code uses older bitmap header / format types which do not support alpha channel or jpg/png compression. On win platforms where these newer formats are available, we should support them so image data isn't lost.

nsImageToClipboard::CreateFromImage -
http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsImageClipboard.cpp#146

nsImageFromClipboard::ConvertColorBitMap -
http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsImageClipboard.cpp#292
I should add, the drag/drop portion of this is specific to the patch for bug 440911 if it lands. Regular drag and drop of files that use the newer stream interfaces do NOT loose image quality. (in fact I'm removing that from the title as it's probably misleading.)
Summary: Copy/paste & drag/drop of images from content area loses transparency information → Copy/paste of images from content area loses transparency information
Summary: Copy/paste of images from content area loses transparency information → Copy/paste of images from content area lose transparency information
This is an experimental patch that adds support for sending alpha channel data in a newer bitmap header format. Unfortunately in testing I found that pretty much every app I worked with ignored the alpha data even though it was present. I might come back to this later and experiment around some more, but at this point I don't see any point in making these changes. We could add support on the receiving end so that apps like thunderbird recognize the info. But in general it would have limited usefulness.
Assignee: jmathies → nobody
Related to bug 249032?
Whiteboard: DUPEME? bug 249032
Assignee: nobody → netzen
As of Bug 670466 we now have a BMP encoder here: mozilla-central\modules\libpr0n\encoders\bmp with support for generating transparent BMPs w/ 32bpp DIBs.  It also does work with other applications.  I think that should be used instead now that we have it.
The fix in bug 555176 will make it so this problem is no longer reproducible.   It adds HTML Format (CF_HTML) to the list of data formats and Thunderbird will use that first instead of having to use CF_DIB which doesn't support transaprency info directly.  

An alternate way to fix this would be to properly support a new flavour in addition to the CF_DIB which is CF_DIBV5 and mark it that way on the clipboard.
Maybe I'm missing something in comment 5, but can't I reproduce this bug by pasting into the Gimp or other graphic editor?
Try right clicking on the image and selecting copy image.  Then paste away.
You can use this sample image:
chrome://branding/content/about-logo.png
Whiteboard: DUPEME? bug 249032
Assignee: netzen → ejpbruel
Depends on: 722555
This patch reimplements the nsImageToClipboard code in terms of the BMP encoder (note that this depends on a patch for bug 722555 that is currently pending review). This allows us to put images on the clipboard using the CF_DIBV5 format, for which the alpha channel is defined.
Attachment #344706 - Attachment is obsolete: true
Attachment #624216 - Flags: review?(netzen)
Attachment #624216 - Attachment is patch: true
This patch updates the nsImageFromClipboard code to take the alpha channel into account when dealing with 32bpp bitmaps (which is what nsImageToClipboard generates). This solves the immediate transparency problem described in the bug.

Eventually we probably want to reimplement the nsImageFromClipboard code to use the BMP decoder. However, unlike the encoder, the decoder doesn't have a public API, and can only be accessed via the image tools API. This is problematic, because the image tools API only takes an input stream as input, and we need to generate a file header in a separate buffer and feed it to the decoder in addition to the image data. This requires the use of a multiplexed image stream.

Such a stream exists, but after some initial attempts, I considered it too much work for this particular bug. We should probably file a separate bug that addresses this issue.
Attachment #624218 - Flags: review?(netzen)
Attachment #624218 - Flags: review?(netzen)
Attachment #624216 - Flags: review?(netzen)
I think you attached the wrong patches. Both seem to be the code for the image refactoring for the v5 header.
Attachment #624216 - Attachment is obsolete: true
Attachment #625531 - Flags: review?(netzen)
Oops. I don't know how I messed up with those patches, but these should be the correct ones. Please take another look at them :-)
Attachment #624218 - Attachment is obsolete: true
Attachment #625532 - Flags: review?(netzen)
Sorry for the delay in reviewing. I want to make sure the approach in bug 722555 is accepted before reviewing this.
(In reply to Brian R. Bondy [:bbondy] from comment #16)
> Sorry for the delay in reviewing. I want to make sure the approach in bug
> 722555 is accepted before reviewing this.

Could you please go ahead and review them anyway? jgilbert is apparently very busy, so the BMP encoder patch is taking a long time to get r+'d. If there are any issues with these patches it would be much more efficient if I can fix those now. rather than wait until the BMP encoder patch gets approved.

We can still wait with landing this stuff until the approach in bug 722555 is accepted, of course (note that jgilbert's first review was a r-, but didn't seem to have issue with the overall approach taken).
I'll have that review done this afternoon, fwiw.
Yup I'll look into this soon, I was only concerned with the v5 header being accepted in general.
Comment on attachment 625532 [details] [diff] [review]
Patch to be reviewed (image from clipboard)

Review of attachment 625532 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsImageClipboard.cpp
@@ +237,2 @@
>  
> +  if (rgbaData) {

You can remove this check since new is infallible by default.
https://developer.mozilla.org/en/Infallible_memory_allocation

@@ +426,5 @@
>              PRUint32 val = *((PRUint32*) (aInputBuffer + index) );
>              aOutBuffer[writeIndex++] = (val & colorMasks.red) >> colorMasks.redRightShift << colorMasks.redLeftShift;
>              aOutBuffer[writeIndex++] =  (val & colorMasks.green) >> colorMasks.greenRightShift << colorMasks.greenLeftShift;
>              aOutBuffer[writeIndex++] = (val & colorMasks.blue) >> colorMasks.blueRightShift << colorMasks.blueLeftShift;
> +            aOutBuffer[writeIndex++] = 0xFF;

It would be better to use the biefields mask here no?

@@ +438,2 @@
>              aOutBuffer[writeIndex++] = aInputBuffer[index];
> +            aOutBuffer[writeIndex++] = aInputBuffer[index+3];

This code assumes that the input format buffer always contain 4 bytes but that is not always the case.  The old code only used the first 3 bytes but then skipped over the 4th byte if it was present.
Attachment #625532 - Flags: review?(netzen)
Comment on attachment 625531 [details] [diff] [review]
Patch to be reviewed (image to clipboard)

Review of attachment 625531 [details] [diff] [review]:
-----------------------------------------------------------------

nsImageToClipboard::CreateFromImage doesn't seem to be working correctly.

I right clicked on an image that was transparent and copied image data, then tried to paste it in mspaint. It had a black background on the transparent parts.

::: widget/windows/nsImageClipboard.cpp
@@ +185,3 @@
>  
> +    PRUint32 size;
> +    encoder->GetImageBufferUsed(&size);

Please check here that size more than BFH_LENGTH you can use NS_ENSURE_TRUE here.

You can also simplify the surrounding code by simply checking NS_ENSURE_SUCCESS(rv, rv).  The advantage of that is it will also log a message to the error console.
Attachment #625531 - Flags: review?(netzen)
Attachment #625531 - Attachment is obsolete: true
Attachment #630256 - Flags: review?(netzen)
Attachment #625532 - Attachment is obsolete: true
Attachment #630265 - Flags: review?(netzen)
Comment on attachment 630256 [details] [diff] [review]
Patch to be reviewed (image to clipboard)

Review of attachment 630256 [details] [diff] [review]:
-----------------------------------------------------------------

Please r=me with the nits addressed.

::: widget/windows/nsImageClipboard.cpp
@@ +158,2 @@
>  
> +    nsCOMPtr<imgIEncoder> encoder = do_CreateInstance("@mozilla.org/image/encoder;2?type=image/bmp", &rv);

nit: Please put the do_CreateInstance on the next line indented an extra 2 spaces from the start of nsCOMPtr.

@@ +190,5 @@
>  
> +    char *dst = (char*) ::GlobalLock(glob);
> +    char *src;
> +    rv = encoder->GetImageBuffer(&src);
> +    NS_ENSURE_SUCCESS(rv, rv);

Move these 3 lines:
char *src;
rv = encoder->GetImageBuffer(&src);
NS_ENSURE_SUCCESS(rv, rv);

above the "GlobalAlloc" and just below "NE_ENSURE_TRUE(size,..." to avoid having to do extra cleanup on failure conditions.
Attachment #630256 - Flags: review?(netzen) → review+
Comment on attachment 630265 [details] [diff] [review]
Patch to be reviewed (image from clipboard)

Review of attachment 630265 [details] [diff] [review]:
-----------------------------------------------------------------

I made a small test program by the way and could extract both data for CF_DIB and CF_DIBV5.
Please r=me with the nits addressed. 
Please push both patches to try and ensure all try tests pass before pushing it out.

Thanks so much for the patches, good work!

::: widget/windows/nsImageClipboard.cpp
@@ +229,5 @@
>    PRInt32 height = header->bmiHeader.biHeight;
>    // neg. heights mean the Y axis is inverted and we don't handle that case
>    NS_ENSURE_TRUE(height > 0, NS_ERROR_FAILURE); 
>  
> +  unsigned char * rgbaData = new unsigned char[width * height * 4 /* RGB */];

nit: /* RGBA */

I know last time I said that the "if (rgbaData)" wasn't needed (and it's not as is), but as I think about it more it is probably best to allocate this with "new (fallible)" instead and then re-add the check and return NS_ERROR_OUT_OF_MEMORY as before.  The reason is because there may be a big image and not enough memory and in that case it would be better not to abort.

@@ +424,2 @@
>              numPixelsLeftInRow--;
>              pos += 4; // we read in 4 bytes of data in order to process this pixel

Please add a comment above this line that BI_BITFIELDS only supports 16bpp and 32bpp so we always add 4 bytes.
Attachment #630265 - Flags: review?(netzen) → review+
Also you have to wait for the image lib changes to land first or at the same time.
Attachment #630256 - Flags: checkin?(netzen)
Attachment #630265 - Flags: checkin?(netzen)
Attachment #630265 - Attachment is obsolete: true
Attachment #630265 - Flags: checkin?(netzen)
Attachment #631139 - Flags: checkin?(netzen)
Waiting to land for the problem to be fixed in bug 722555 Comment 30.
Comment on attachment 630256 [details] [diff] [review]
Patch to be reviewed (image to clipboard)

Dependent patch needs to land yet, see bug 722555 Comment 30.
Attachment #630256 - Flags: checkin?(netzen) → checkin-
Comment on attachment 631139 [details] [diff] [review]
Patch to be committed (image from clipboard)

Dependent patch needs to land yet, see bug 722555 Comment 30.
Attachment #631139 - Flags: checkin?(netzen) → checkin-
Comment on attachment 631139 [details] [diff] [review]
Patch to be committed (image from clipboard)

Dependent patch just landed :-)
Attachment #631139 - Flags: review?(netzen)
Attachment #631139 - Flags: review?(netzen)
Attachment #631139 - Flags: review+
Attachment #631139 - Flags: checkin-
Attachment #631139 - Flags: checkin+
Attachment #630256 - Flags: checkin- → checkin+
Comment on attachment 631139 [details] [diff] [review]
Patch to be committed (image from clipboard)

(In reply to Eddy Bruel [:ejpbruel] from comment #34)
> http://hg.mozilla.org/integration/mozilla-inbound/rev/31f42dd24f25

This changeset backed out for mochitest-other failures in test_bug444800.xul:
https://tbpl.mozilla.org/php/getParsedLog.php?id=14004465&tree=Mozilla-Inbound
{
34246 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/widget/tests/test_bug444800.xul | an unexpected uncaught JS exception reported through window.onerror - NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsITransferable.getAnyTransferData] at chrome://mochitests/content/chrome/widget/tests/test_bug444800.xul:64 
}

https://hg.mozilla.org/integration/mozilla-inbound/rev/2d7c387f47ec
Attachment #631139 - Flags: checkin+ → checkin-
(In reply to Ed Morley [:edmorley] from comment #35)
> Comment on attachment 631139 [details] [diff] [review]
> Patch to be committed (image from clipboard)
> 
> (In reply to Eddy Bruel [:ejpbruel] from comment #34)
> > http://hg.mozilla.org/integration/mozilla-inbound/rev/31f42dd24f25
> 
> This changeset backed out for mochitest-other failures in test_bug444800.xul:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=14004465&tree=Mozilla-
> Inbound
> {
> 34246 ERROR TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/chrome/widget/tests/test_bug444800.xul | an
> unexpected uncaught JS exception reported through window.onerror -
> NS_ERROR_FAILURE: Component returned failure code: 0x80004005
> (NS_ERROR_FAILURE) [nsITransferable.getAnyTransferData] at
> chrome://mochitests/content/chrome/widget/tests/test_bug444800.xul:64 
> }
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/2d7c387f47ec

I'm sorry, that was careless. I ran the patch on try before I pushed it but had assumed it was a known intermittent. Obviously its not. I'll look into it.
No problem. In cases where the intermittent is known, there should be bugs suggested by TBPL. If there aren't, you've either been extremely unlucky to get a new (as yet unfiled) intermittent, or it is real :-)

A debug build has just finished - here's the log, in case it helps you debug more easily:
https://tbpl.mozilla.org/php/getParsedLog.php?id=14005151&tree=Mozilla-Inbound
https://hg.mozilla.org/mozilla-central/rev/0a570be19457
https://hg.mozilla.org/mozilla-central/rev/4fa21b477540
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Depends on: 787741
Depends on: 787769
Depends on: 816530
Comment on attachment 631139 [details] [diff] [review]
Patch to be committed (image from clipboard)

I'm confused - in light of bug 816530, it appears that the image-from-clipboard part was backed out but there was no action since. Has this ever been fixed?
(In reply to rsx11m from comment #39)
> Comment on attachment 631139 [details] [diff] [review]
> Patch to be committed (image from clipboard)
> 
> I'm confused - in light of bug 816530, it appears that the
> image-from-clipboard part was backed out but there was no action since. Has
> this ever been fixed?

It does look like this is the case.

Indeed, this was backed out here:
http://hg.mozilla.org/mozilla-central/rev/2d7c387f47ec
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: ejpbruel → nobody
Is progress likely to be made on this issue?

It seems to have stalled since #816530.

I think in a year like 2019, it is a good idea to pick this up again, as Firefox is the only modern browser left with this behavior and copy/pasting, especially transparent, images is a convenient part of everyday browsing.

See Also: → 1527235
Priority: -- → P3
Whiteboard: widget-next

This is the simple repro page I created, right-click -> Copy Image and ctrl+V it back onto the page to see how the background becomes black.

If I right-click > Copy Image on a PNG with transparency and then paste into Windows 10 Paint 3D, the transparency is preserved. However, this is not true when pasting into a website (for example, Twitter). If we assume the data on the clipboard is correct, what needs to be done to preserve transparency when accessing a file from a ClipboardEvent.clipboardData (DataTransfer) object?

I'm so sorry to needinfo you, but is this under the wrong component? If 1520656 was under ImageLib then should this component be there as well? I don't seem to be able to change the component myself.

Flags: needinfo?(ehumphries)

Thanks, vaindil. I am not sure of this but asking the person who filed this bug if it should be moved.

:jimm, this still a Widget::Win 32 issue or should it move to ImageLib?

Flags: needinfo?(ehumphries) → needinfo?(jmathies)

(In reply to Emma Humphries, Bugmaster ☕️🎸🧞‍♀️✨ (she/her) [:emceeaich] (UTC-8) needinfo? me from comment #47)

Thanks, vaindil. I am not sure of this but asking the person who filed this bug if it should be moved.

:jimm, this still a Widget::Win 32 issue or should it move to ImageLib?

This feels like a clipboard bug down in widget. I'll see if I can get someone to take a look.

Flags: needinfo?(jmathies)
Priority: P3 → P2

Visualization of image transparency differs between Google Chrome and Firefox:
https://www.rgk4it.com/wp-content/uploads/2019/08/logo.png

Is it related to the way of firefox do image render?

This feels like a clipboard bug down in widget. I'll see if I can get someone to take a look.

Relevant Stackoverflow
https://stackoverflow.com/questions/44177115/copying-from-and-to-clipboard-loses-image-transparency

Windows clipboard transparency is a mess.

I intend to investigate this further along with the other image clipboard / color management work in the meta bug.

I am still experiencing this on Windows in Firefox 93.

Status: REOPENED → NEW
Target Milestone: mozilla17 → ---
Whiteboard: widget-next → widget-next,[win:pasteboard]
Severity: normal → S3

The severity field for this bug is relatively low, S3. However, the bug has 5 duplicates and 10 votes.
:spohl, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(spohl.mozilla.bugs)

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Flags: needinfo?(spohl.mozilla.bugs)
Duplicate of this bug: 1799667

As a web-based graphic designer this bug is crippling and the reason why I keep going back to Chrome :(

Duplicate of this bug: 1664387

Yeah we should fix this.

Severity: S3 → S2

Back when this was filed, we used a completely different way to encode BMPs for the clipboard. Since bug 1501482, we now use our standard BMP image encoder. That should properly support transparency.

I suspect part of this is we don't request v5 of BMP images when we drag/drop:
https://searchfox.org/mozilla-central/rev/2fc2ccf960c2f7c419262ac7215715c5235948db/widget/windows/nsDataObj.cpp#1622

That can probably just be changed and improve things. Probably we don't ever want to provide v3 version here too:
https://searchfox.org/mozilla-central/rev/2fc2ccf960c2f7c419262ac7215715c5235948db/widget/windows/nsDataObj.cpp#994

I'm happy to attempt to fix this if you don't have time. I'm glad we already have v5 DIB code as that makes this kind of trivial.

Assignee: nobody → ahale

(In reply to Jeff Muizelaar [:jrmuizel] from comment #64)

It looks like we can/should also write PNG data to the clipboard: https://source.chromium.org/chromium/chromium/src/+/main:ui/base/clipboard/clipboard_win.cc;l=726;drc=d030a17ad0ce961375e5e8d47cdc3e570b5a8fab

Yeah that would be a big improvement, see bug 1717306.

For bug 1717306 I would be interested in some Windows applications that currently have problems when copying images from/to Firefox and are free to install.

I'm working on this bug, my understanding is that we want to:

  • setting clipboard - make Firefox offer PNG and BMPv5 on the clipboard, possibly also BMPv1 if any apps fail to use BMPv5.
  • getting clipboard - make Firefox load PNG and BMPv5 on the clipboard with transparency support.

Current status I gather from this bug and the links on it and my browsing of the code:

  • setting clipboard:
    • currently Firefox offers BMPv3 on the clipboard, the alpha channel is technically written but ignored by apps loading BMPv3.
    • currently Firefox does not offer PNG on the clipboard, which is required by some apps.
  • getting clipboard:
    • currently Firefox loading BMPv5 does respect the alpha channel (?) according to some of the reports (e.g. copying in Chrome and pasting in Firefox).
    • currently Firefox does not attempt to load PNG from the clipboard (?) or does not respect transparency on it (?).

I'm going to test with a few apps to get a better idea of the current status of these assumptions, and verify if changes fix them. I'll post my findings as comments.

As a first test, I changed the code to offer BMPv5 instead of BMPv3 on the clipboard, and using chrome://branding/content/about-logo.png as a test image for copying with the context menu 'Copy Image' option, I tried pasting into the following:

  • GIMP - pastes with transparency (correct), so GIMP appears to be optimistic about transparent imagery in BMP formats.
  • LibreOffice Writer - pastes as opaque (incorrect)
  • Google Docs in Firefox - pastes as opaque (incorrect), this is a problem with Firefox loading from the clipboard.
  • Google Docs in Edge - pastes as transparent (correct)
  • Windows clipboard browser (Win-V shortcut) - can't tell either way, always previews images as opaque, not a useful tool for testing.

So I don't think there is currently any problem with how Firefox posts BMP to the clipboard, but some apps need PNG.

I'll look into why Firefox is ignoring the alpha channel on load now (the Google Docs in Firefox case).

Debugging the clipboard code, we're correctly reading the BMPv5 from the clipboard as transparent, but then encoding it as a PNG to deliver to Google Docs, and that encoding of the PNG has the transparent flag set and all, but if I dump the memory to disk using WinDbg .memdump I get an image that when loaded in GIMP says it is RGB color. So there's something going wrong with creating the PNG image internally.

This is the invocation of the nsPNGEncoder https://searchfox.org/mozilla-central/rev/a44deb21744de6269329b05461c55488a147f95e/widget/windows/nsClipboard.cpp#730 and I'm not sure there is anything wrong with the options we're passing (useTransparency is on by default in the nsPNGEncoder code https://searchfox.org/mozilla-central/rev/a44deb21744de6269329b05461c55488a147f95e/image/encoders/png/nsPNGEncoder.cpp#90 ).

Looking at the mPNGInfo struct in the debugger:

width 192
height 192
valid 0
rowbytes 768
palette NULL
num_palette 0
num_Trans 0
bit_depth 8
color_type 6  (this represents COLOR and ALPHA flags)
compression_type 0
filter_type 0
interlace_type 0
channels 4
pixel_depth 32
...
trans_alpha 0
trans_color (all zeros)
...

I'm not quite familiar enough with the PNG format to see what is wrong with this mPNGInfo but I am guessing we're not quite correctly flagging it as transparent.

currently Firefox offers BMPv3 on the clipboard, the alpha channel is technically written but ignored by apps loading BMPv3.

I am a bit confused, because I was pretty sure that Firefox writes BMPv5 and BMPv3 to the clipboard. https://searchfox.org/mozilla-central/rev/a44deb21744de6269329b05461c55488a147f95e/widget/windows/nsClipboard.cpp#258-263

Thanks, yeah I found the BMPv5 code when I actually debugged it. The codebase has a lot of BMP writers and I found two that were not the relevant ones in my initial research but the debugger hit the right ones :)

So in further debugging/editing, I've managed to get the nsBMPDecoder to cooperate with transparent BMP on the clipboard, at least from some apps (including Firefox), so I can freely copy from Firefox to Firefox, from Firefox to GIMP, from Firefox to Edge, but not GIMP to Firefox which seems to have something meaningfully different about the BMP data it posts on the clipboard, whereas GIMP to Edge works fine, so there's something I don't understand yet about how GIMP's clipboard objects work.

No longer blocks: gfx-triage
Depends on: 1805330

In my local repo I have several fixes coded, and will be sending the stack of diffs out for code review next week.

Status:

  • Copying to clipboard now posts as the format identifier ::RegisterClipboardFormatW(L"PNG") (which seems not entirely successful - still debugging our internal format conversions which is skipping over this) and CF_DIBV5 and CF_DIB, now the CF_DIBV5 is posted with Compression::BITFIELDS which works well for some apps (including Firefox) that understand this more advanced format which gives us a better opportunity to hint there is intentionally an alpha channel in the image (which Firefox already understood), the CF_DIB is still Compression::RGB for best compatibility (but apps reading this will almost always drop the alpha channel).
  • Pasting from clipboard now attempts to handle ::RegisterClipboardFormatW(L"PNG") and CF_DIBV5 and CF_DIB, both CF_DIBV5 and CF_DIB with Compression::RGB are interpeted with an assumption the alpha channel is valid, this could theoretically break but I have yet to see an app post garbage in the alpha channel so it most likely works broadly, it treats images that are entirely 0 alpha as entirely opaque for broadest compatibility (same logic we use when reading DIB images in ICO files).
  • Dragging an image from Firefox writes the file as BMPV5 with Compression::BITFIELDS working for drag and drop (this is where it gets more peculiar as BMPV5 with BITFIELDS is a relatively newer and more advanced format, but it's our only opportunity to clearly hint that the alpha channel exists using BMP format), it would likely be better to write the file as PNG as BMP so I'll be trying to get that working too.
  • Dragging an image into Firefox already works with multiple formats, so not much to do there.
Depends on: 1717306

Since I've been adding PNG support (both copying and pasting) I've added Bug 1717306 as a dependency as my patches will likely resolve that as well.

OS: Windows XP → Windows

Ashley, are you still working on this? "will be sending the stack of diffs out for code review next week" was a while ago ;-)

Flags: needinfo?(ahale)

I'm aiming to land the majority of the code I wrote in https://phabricator.services.mozilla.com/D164531 this week, but I need to remove the pref clipboard.copy_image_as_png_format as it isn't working.

My progress on this bug has been blocked on an issue with the code for the new pref clipboard.copy_image_as_png_format - we have a rather arcane set of internal abstractions for interacting with the clipboard, and despite registering a png data callback it never gets called, so the pref doesn't work, and it's key to resolving this bug, but I do have a number of other fixes and improvements pending in that diff so I will try to land them.

My current plan for the code in the diff is to:

  • Split out the pref clipboard.copy_image_as_png_format and associated code into its own bug and diff.
  • Change the pref definitions to enable by default in nightly for:
    • clipboard.paste_image_support_bmp_transparency_with_rgb_compression
    • clipboard.copy_image_as_bmpv5_using_bitfields_compression
    • clipboard.drag_and_drop_images_as_png
  • Keeping the default-off setting for pref clipboard.drag_and_drop_images_as_bmpv5_using_bitfields_compression, but the code will be there. This pref causes our drag and drop image posting to be bmpv5 with bitfields compression - it's likely that a lot of apps would not understand this format, and it's arguably better to use png (hence that pref being on by default instead).

It feels bad to land it without clipboard.copy_image_as_png_format but until I get more help debugging that I can't really proceed on that fix.

Depends on: 1832396

I got the PNG copy/paste to work so I'll be trying to land https://phabricator.services.mozilla.com/D177868 before fussing more about bmp handling as it mostly or completely resolves this bug.

Duplicate of this bug: 1835630

(In reply to Ashley Hale [:ahale] from comment #77)

I got the PNG copy/paste to work so I'll be trying to land https://phabricator.services.mozilla.com/D177868 before fussing more about bmp handling as it mostly or completely resolves this bug.

What is the status of that patch?

Blocks: 1866655
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: