Status

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: philip.chee, Assigned: philip.chee)

Tracking

SeaMonkey 2.50 Branch
seamonkey2.50
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.50 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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)
Depends on: 1322609
No longer depends on: 1323968
bustage fix :P
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #8819449 - Flags: review?(iann_bugzilla)
Attachment #8819449 - Flags: feedback?(frgrahl)
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".
(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");
Attachment #8819449 - Flags: review?(iann_bugzilla)
Attachment #8819449 - Flags: feedback?(frgrahl)
Fix tabbrowser mistakes in previous patch.
Attachment #8819449 - Attachment is obsolete: true
Attachment #8820436 - Flags: review?(iann_bugzilla)
Attachment #8820436 - Flags: feedback?(frgrahl)
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+.
Attachment #8820436 - Flags: feedback?(frgrahl) → feedback+
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.
Attachment #8820436 - Flags: review?(iann_bugzilla) → review+
https://hg.mozilla.org/comm-central/rev/7f6a92e78b37d9f05e8a482aa6093c2aa6d57dcc
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.50
You need to log in before you can comment on or make changes to this bug.