copying text on cbc.ca (and newyorker.com, other sites using tynt) is broken

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
Serializers
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: dria, Assigned: mats)

Tracking

({regression})

Trunk
mozilla2.0b7
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

(URL)

Attachments

(2 attachments, 3 obsolete attachments)

If you go to cbc.ca with today's nightly, select, and try to copy text, it ...doesn't.  Works ok in 3.6, although cbc uses one of those insanely annoying scripts that appends additional content to whatever you're copying.

Turning off javascript.options.tracejit.content and javascript.options.methodjit.content didn't fix it (beltzner's suggestion).

Specific URL I was trying to copy from:

http://www.cbc.ca/technology/story/2010/10/06/tech-hidden-language.html?ref=rss
Deb, any idea what the regression range is?
Keywords: regression, regressionwindow-wanted
I tested /pub/mozilla.org/firefox/nightly/2010-09-05-03-mozilla-central mac64 and it was happening there then, too...robcee suggests it might be UA string related.

Comment 3

8 years ago
I can confirm this bug as well on other sites that use Tynt  (newyorker.com, smithsonian.com, sportsillustrated.com). I didn't notice this earlier because I forgot I was actively blocking that script. :/
Neil, do you happen to have a URI for this Tynt thing?
Keywords: qawanted

Comment 5

8 years ago
Here's the direct URL to the script we're calling off of the CBC site:

http://tcr.tynt.com/javascripts/Tracer.js?user=cT9yCKGeer3PWlab7jrHtB&s=62
(In reply to comment #2)
> robcee suggests it might be UA string related.

Just changed my UA string to match 3.6.9 by manually overriding it via about:config, and that had no effect. Seems to actually be a JS or perhaps a clipboard bug?
Regression range would help. I cc'd Jeff because it smelled a bit like an ES5 compat issue to me.
This is actually a rather old regression. I started bisecting tracemonkey, but the result of that just included a mozilla-central merge, so then I bisected that:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=da9e50cb4091&tochange=ffbc3baf03ae
https://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=b3e27c1ee35e&tochange=4bed87d68dab

Bug 39098 is in both of those, and I think it's more likely to be the cause than a JS engine bug.
Assignee: general → nobody
Component: JavaScript Engine → General
Keywords: regressionwindow-wanted
QA Contact: general → general
Blocks: 39098
blocking2.0: --- → ?
I confirmed that this was caused by bug 39098.
Summary: copy on cbc.ca is broken in nightlies → copying text on cbc.ca (and newyorker.com, other sites using tynt) is broken
(Assignee)

Comment 10

8 years ago
Created attachment 481417 [details]
Indented version of Tracer.js (in comment 5)
(Assignee)

Comment 11

8 years ago
Created attachment 481425 [details]
Indented version of Tracer.js with line numbers (in comment 5)

Tracer.js does something like this:
line 436: create a DIV (d)
line 451-459: append the current selection to it (along with some other stuff)
line 478: create another DIV (e)
line 479: append that to BODY (k)
line 480: append (d) to (e)
line 481: create a range (a)
line 482: add (d) to it
line 483: remove the current selection
line 484: select the range (a)
line 485: setup a timer that will restore the original selection range

This all happens in an event handler (oncopy I think), and when our
internal copy-to-clipboard command runs it sees the "fake" selection
with the (a) range.

The inserted DIV (e) does not have a frame when doing the copy,
so it's invisible and results in an empty string.
The node has a NODE_NEEDS_FRAME flag though, so we could
either check that (on any ancestor) or we could do a flush
(creating frames) before starting the copy somehow.
(Assignee)

Updated

8 years ago
Keywords: qawanted
(Assignee)

Comment 12

8 years ago
Flushing seems more correct, I'll try that first...
Assignee: nobody → matspal
Component: General → Serializers
OS: Mac OS X → All
QA Contact: general → dom-to-text
Hardware: x86 → All

Updated

8 years ago
blocking2.0: ? → final+
(Assignee)

Updated

8 years ago
Attachment #481417 - Attachment is obsolete: true
(Assignee)

Comment 13

8 years ago
Created attachment 481752 [details] [diff] [review]
Patch rev. 1
Attachment #481752 - Flags: review?(bzbarsky)
Comment on attachment 481752 [details] [diff] [review]
Patch rev. 1

Do you need to worry if the flush destroys the presshell?  r=me if not; otherwise you need another IsDestroying check.
Attachment #481752 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 15

8 years ago
Created attachment 481987 [details] [diff] [review]
Patch rev. 2

Good point, I think that it can happen.
Attachment #481752 - Attachment is obsolete: true
Attachment #481987 - Flags: review?(bzbarsky)
Comment on attachment 481987 [details] [diff] [review]
Patch rev. 2

Hmm, one more thing.  Do you need a layout flush or a frame flush?  That is, does clipboard care about geometry, or just the styles?
(Assignee)

Comment 17

8 years ago
Created attachment 481996 [details] [diff] [review]
Patch rev. 3

It doesn't use geometry, it's determined from the presence of a frame and
its visibility style.  So, as you suggest, a more limited flush should
be enough.  Flush_Frames is the right choice?
Attachment #481996 - Flags: review?(bzbarsky)
(Assignee)

Updated

8 years ago
Attachment #481987 - Attachment is obsolete: true
Attachment #481987 - Flags: review?(bzbarsky)
Comment on attachment 481996 [details] [diff] [review]
Patch rev. 3

Yep, if you check for frame presence Flush_Frames is good.
Attachment #481996 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 19

8 years ago
http://hg.mozilla.org/mozilla-central/rev/9f62edc36b29
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.