1250958 - fix browser_markPageAsFollowedLink.js and framed link coloring for e10s

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Marco Bonardo [:mak]]

skip-if = e10s # Bug 933103 - mochitest's EventUtils.synthesizeMouse functions not e10s friendly (test does EventUtils.sendMouseEvent...)
51 [browser_sidebarpanels_click.js]

Comment 1

[Marco Bonardo [:mak]]

Attachment: MozReview Request: Bug 1250958 - fix browser_markPageAsFollowedLink.js for e10s (and fix framed link coloring to make it pass). r=jaws

Review commit: https://reviewboard.mozilla.org/r/36887/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36887/

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8724141 [details]
MozReview Request: Bug 1250958 - fix browser_markPageAsFollowedLink.js for e10s (and fix framed link coloring to make it pass). r=jaws

https://reviewboard.mozilla.org/r/36887/#review33825

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e427eb78d33b

Comment 4

[Marco Bonardo [:mak]]

backed out, failing on linux/mac
https://hg.mozilla.org/integration/fx-team/rev/5048f088aedf
investigating.

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 8724141 [details]
MozReview Request: Bug 1250958 - fix browser_markPageAsFollowedLink.js for e10s (and fix framed link coloring to make it pass). r=jaws

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36887/diff/1-2/

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

https://reviewboard.mozilla.org/r/36887/#review33979

Don't know how I didn't catch that in the first review.

Comment 7

[Marco Bonardo [:mak]]

https://reviewboard.mozilla.org/r/36887/#review33979

honestly, it's still failing on Try, there's something more fishy here.

Comment 8

[Marco Bonardo [:mak]]

ah I think the problem is that I misinterpreted the includeSubframes argument of browserLoaded, thinking setting it to true would have waited for subframes, but it's rather the opposite.

Comment 9

[Marco Bonardo [:mak]]

Comment on attachment 8724141 [details]
MozReview Request: Bug 1250958 - fix browser_markPageAsFollowedLink.js for e10s (and fix framed link coloring to make it pass). r=jaws

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36887/diff/2-3/

Comment 10

[Marco Bonardo [:mak]]

The last version seems to be stable enough.
I decided to simplify the change to contentClick, by just moving the code up some rows, so markPage is always invoked before the click.
Changed to BrowserTestUtils.openNewForegroundTab and added a promiseAsyncUpdates to be sure db transactions are done (looks like we may end up notifying before the transaction is complete, that means in some rare case the concurrent connection could not see the change).

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa76cb4f1bd9&filter-resultStatus=success&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=runnable&group_state=expanded

In the end the changes are uninteresting and don't change the essence of the patch, so I'm not going to reflag. If something's wrong please let me know.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/51dff7d577ea

Comment 12

[Marco Bonardo [:mak]]

[Tracking Requested - why for this release]: may be worth to fix this in firefox 46, cause framed links coloring is currently broken on e10s

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/51dff7d577ea

Comment 14

[Marco Bonardo [:mak]]

Comment on attachment 8724141 [details]
MozReview Request: Bug 1250958 - fix browser_markPageAsFollowedLink.js for e10s (and fix framed link coloring to make it pass). r=jaws

Approval Request Comment
[Feature/regressing bug #]: e10s
[User impact if declined]: links across frames won't get proper visited link coloring
[Describe test coverage new/current, TreeHerder]: unit test
[Risks and why]: low risk, simple change
[String/UUID change made/needed]: none

Comment 15

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Regression from e10s, tracking for 46+.

Comment 16

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8724141 [details]
MozReview Request: Bug 1250958 - fix browser_markPageAsFollowedLink.js for e10s (and fix framed link coloring to make it pass). r=jaws

Link color should work across frames, let's uplift this and verify it in beta 46.

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fc1d39a3fe5f

Comment 18

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected