1324121 - Port bug 1322609 and bug 1322414 to suite

Status: RESOLVED FIXED

Description

[Philip Chee]

mozilla-central bugs:
Bug 1322609 - Get rid of content-targetable usage in tabbrowser.xml
Bug 1322414 - Move content vs. content-primary distinction out of the `type` browser attribute

Thunderbird bug:
Bug 1323968 (Port bug 1322609 and bug 1322414 to mail/ and editor/)
(TB (2016-12-16) unusable. No entries in folderPane)

Comment 1

[Philip Chee]

Attachment: rename content-primary and rename content-targetable

bustage fix :P

Comment 2

[Jorg K (GMT+2)]

Please note that with the patch in bug 1323968:
https://hg.mozilla.org/comm-central/rev/42aa2df379ce3ffa520f01c2799a9e4283c45327
we are still seeing a whole heap of Mozmill failures:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8afa0892704bf83bc0168407fa71abf6234718c9

Comment 3

[Jorg K (GMT+2)]

Comment on attachment 8819449 [details] [diff] [review]
rename content-primary and rename content-targetable

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

::: suite/browser/tabbrowser.xml
@@ -1138,5 @@
>              newBrowser.docShellIsActive = this.mCurrentTab.linkedBrowser.docShellIsActive;
>              if (this.mCurrentBrowser) {
>                this.mCurrentBrowser.droppedLinkHandler = null;
>                this.mCurrentBrowser.docShellIsActive = false;
> -              this.mCurrentBrowser.setAttribute("type", "content-targetable");

Why is this OK to remove? Just curious. We changed "content-targetable" to "content".

Comment 4

[Philip Chee]

(In reply to Jorg K (GMT+1) from comment #3)
>> -              this.mCurrentBrowser.setAttribute("type", "content-targetable");
> 
> Why is this OK to remove? Just curious. We changed "content-targetable" to
> "content".
Somewhere else we used to set type to "content-targetable" now that we don't we also don't need to remove it

> -            // We are no longer a targetable content area
> -            oldBrowser.setAttribute("type", "content");

Comment 5

[Philip Chee]

Attachment: rename content-primary and rename content-targetable (patch that works)

Fix tabbrowser mistakes in previous patch.

Comment 6

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8820436 [details] [diff] [review]
rename content-primary and rename content-targetable (patch that works)

Didn't have problems with the previous patch and the new one also works for me. Looks like i just didn't hit any problems with my daily brosing habits. Based on a diff between both patches the changes look reasonable so f+.

Comment 7

[Ian Neal]

Comment on attachment 8820436 [details] [diff] [review]
rename content-primary and rename content-targetable (patch that works)

>+++ b/suite/browser/tabbrowser.xml
>-            <xul:browser flex="1" type="content-primary" xbl:inherits="tooltip=contenttooltip,contextmenu=contentcontextmenu,autocompletepopup"/>
>+            <xul:browser flex="1" type="content" primary="true"
>+                         xbl:inherits="tooltip=contenttooltip,contextmenu=contentcontextmenu,autocompletepopup"/>
Nit: one attribute per line.

>-            // We are no longer a targetable content area
>-            oldBrowser.setAttribute("type", "content");
>+            // We are no longer the primary content area
Nit: full stop at the end of the comment.
>+            oldBrowser.removeAttribute("primary");

r/a=me with those addressed.

Comment 8

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/comm-central/rev/7f6a92e78b37d9f05e8a482aa6093c2aa6d57dcc