"Extension"/"Plugins" pane of "Add-ons Manager" is drawn wrongly if zoom level not equal 100% in site specific zoom and disabled extension/plugin existed.

VERIFIED FIXED in mozilla2.0b10

Status

()

defect
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: alice0775, Assigned: dholbert)

Tracking

({regression})

Trunk
mozilla2.0b10
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [hardblocker](?)[fx4-fixed-bugday])

Attachments

(5 attachments, 1 obsolete attachment)

Reporter

Description

9 years ago
No description provided.
Reporter

Comment 1

9 years ago
Sample extentions folder.
Note:
Containing extensions are disabled by default because of maxVersion less tan 4.0b9pre.
Reporter

Comment 2

9 years ago
Build Identifier:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101222 Firefox/4.0b9pre ID:20101222030351

"Extension" pane of "Add-ons Manager" is drawn wrongly  if zoom level does not equal 100%  in site specific zoom.


Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Open Add-ons Manager(Ctrl+Shift+A)
3. Zoom out (Ctrl++ Ctrl++ Ctrl++)
4. Install Several Addons
4'.Alternative, Exit Browser and Copy extensions folder(in the attached zip) to profile folder. And Restart browser
5. Open Add-ons Manager(Ctrl+Shift+A)

Actual Results:
 "Extension" pane of "Add-ons Manager" is drawn wrongly

Expected Results:
 "Extension" pane of "Add-ons Manager" is drawn in specified zoom level

Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/a2a3a6e8b0e0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101221 Firefox/4.0b9pre ID:20101221145009
Fails:
http://hg.mozilla.org/mozilla-central/rev/52b229f6a051
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101221 Firefox/4.0b9pre ID:20101221150740
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a2a3a6e8b0e0&tochange=52b229f6a051
Reporter

Updated

9 years ago
Keywords: regression
Reporter

Comment 3

9 years ago
Posted image screenshot
Reporter

Updated

9 years ago
Summary: "Extension" pane of "Add-ons Manager" if zoom levbel not equal 100% in site specific zoom. → "Extension" pane of "Add-ons Manager" is drawn wrongly if zoom levbel not equal 100% in site specific zoom.
Reporter

Comment 4

9 years ago
reduced extensions
Attachment #499618 - Attachment is obsolete: true
Reporter

Comment 5

9 years ago
It happens if disabled extention/plugin existed in the Manager.
Summary: "Extension" pane of "Add-ons Manager" is drawn wrongly if zoom levbel not equal 100% in site specific zoom. → "Extension"/"Plugins" pane of "Add-ons Manager" is drawn wrongly if zoom levbel not equal 100% in site specific zoom and disabled extention/plugin existed.
Reporter

Comment 6

9 years ago
It happens on Linux too.
Mozilla/5.0 (X11; Linux i686; rv:2.0b9pre) Gecko/20101223 Firefox/4.0b9pre ID:20101223030401


In local build.
Build from 6b98f6cf4589 : fails
Build from 95bbe1076dcb : fails
Build from a2a3a6e8b0e0 : works:

Regressed by 95bbe1076dcb	Daniel Holbert — Bug 611241: Allow SVG filters from external documents to be used in about: pages. r=bzbarsky a=blocking-betaN+
Blocks: 611241
blocking2.0: --- → ?
Component: Layout → SVG
OS: Windows 7 → All
QA Contact: layout → general
Reporter

Updated

9 years ago
Summary: "Extension"/"Plugins" pane of "Add-ons Manager" is drawn wrongly if zoom levbel not equal 100% in site specific zoom and disabled extention/plugin existed. → "Extension"/"Plugins" pane of "Add-ons Manager" is drawn wrongly if zoom levbel not equal 100% in site specific zoom and disabled extension/plugin existed.
Reporter

Comment 7

9 years ago
This happens on attachment 490778 [details] of Bug 611241 too.

On latest nightly
  Open about:bug611241 and zoom In
  Reload --- zoom reset, NG
  Switch tabs --- zoom level as expected

On build prior to landing of Bug 611241 (Of course the first image is not displayed)
  Open about:bug611241 and zoom In
  Reload and  Switch tabs--- no problem
This sounds like it may be related to bug 541270 (although it looks a bit different).
Assignee

Updated

9 years ago
Summary: "Extension"/"Plugins" pane of "Add-ons Manager" is drawn wrongly if zoom levbel not equal 100% in site specific zoom and disabled extension/plugin existed. → "Extension"/"Plugins" pane of "Add-ons Manager" is drawn wrongly if zoom level not equal 100% in site specific zoom and disabled extension/plugin existed.

Comment 9

9 years ago
At least on XP; The full page displays at the correct zoom level first, then the inset (circled area) is displayed after that.
Then if you get the tab to redisplay (ex: press Reload button; switch off tab, then go back...), what is left is what Alice screenshot shows.

Also if you look closely at the image, the larger correctly zoomed screen does not have the images displayed that Bug 611241 fixed.
 So it looks like code that displays the about:addons (without images) executes first, the code from Bug 611241 comes and overlays it with a non-zoomed page that only uses only what 100% would use off the zoomed level (ex: if your zoom level for the Add-ons page is set to 200% that displays first then a screen 1/2 its size displays over it).
 In the attached example I have the Zoom level set to 140%.
Assignee: nobody → dholbert
blocking2.0: ? → final+
Assignee

Comment 10

9 years ago
I can reproduce this using the Simple Testcase extension, attached to bug 611241. (at both the about: and chrome: URLs in bug 611241 comment 4)
Assignee

Comment 11

9 years ago
(In reply to comment #8)
> This sounds like it may be related to bug 541270 (although it looks a bit
> different).

Yeah, I think they're roughly the same bug.
Assignee

Comment 12

9 years ago
Or at least that bug is part of the brokenness that now affects about:addons.  The brokenness on tab-switch & reload (not described in that bug) may be xul-specific.
Assignee

Comment 13

9 years ago
The reload & tab-switch issues in this bug (the most broken part) only happens for content with a filter defined in an external resource.  So it happens for about:addons (which outsources its filter), but not for the testcases on bug 541270 (which use locally-embedded filters).

I've filed bug 621639 on the reload & tab-switch issues, with a reduced XHTML testcase.

IIUC, that's the main thing that's busted here.  (The part covered by bug 541270 actually just means that the icons for disabled addons will be cropped when you zoom out, and that's not as annoying of a problem.)

So since bug 621639 is specific to external resources, we could *almost* hack around it for about:addons by using a locally-defined filter.  However, that workaround can't actually work for about:addons, because local references are busted for about: pages, as bz noted in bug 611241 comment 5.
Depends on: 621639
Whiteboard: [hardblocker](?)
Assignee

Updated

9 years ago
No longer depends on: 621639
Duplicate of this bug: 621639
Assignee

Comment 15

9 years ago
(In reply to comment #13)
> I've filed bug 621639 on the reload & tab-switch issues, with a reduced XHTML
> testcase.
> IIUC, that's the main thing that's busted here.

bug 621639 is indeed the cause of the issue from comment 2 here -- so I just duped bug 621639 back to this bug.

I have a WIP patch -- coming up soon.
Assignee

Comment 16

9 years ago
Posted patch fixSplinter Review
This patch sets the External Resource's zoom levels to match those of its display document *when it's created*.

This change is necessary because the initial calls to DocumentViewerImpl::SetFullZoom() & SetTextZoom() for the display document happen *before* our external resources are loaded, and so their well-meaning "EnumerateExternalResources()" chunks don't actually accomplish anything (as far as this bug is concerned) at page-load time. (See bug 621639 comment 6 & the comments leading up to it for more.)
Attachment #502706 - Flags: review?(bzbarsky)
Assignee

Updated

9 years ago
Flags: in-testsuite?
Assignee

Comment 17

9 years ago
The patch includes a reftest based on the testcase in simplified/duplicate bug 621253.  It just compares full-page-zoom on content with an external filter vs. content with a locally-defined filter.  I confirmed with a local reftest-run that it fails pre-patch & passes post-patch.
Assignee

Updated

9 years ago
Status: NEW → ASSIGNED
Hardware: x86 → All
Comment on attachment 502706 [details] [diff] [review]
fix

r=me.  Sorry for the lag...
Attachment #502706 - Flags: review?(bzbarsky) → review+
Assignee

Comment 19

9 years ago
No problem, thanks for the review!
Landed: http://hg.mozilla.org/mozilla-central/rev/0609193325c9
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: mozilla2.0 → mozilla2.0b10
Assignee

Comment 20

9 years ago
backed out due to crashes during unittest runs.  Looks like one or the other document is null.  (I didn't think either could be, but I guess so (?)):
http://hg.mozilla.org/mozilla-central/rev/9ab122e89d62

Probably just needs a null-check somewhere, but I want to understand why the document's null before adding the null-check...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 21

9 years ago
Aha! So earlier I'd had the "TransferZoomLevels()" call up inside of the "if (aViewer)" clause, where we set |doc| and then proceed assuming it's non-null.

However, once we've left that clause, we no longer no for sure that |doc| (the external resource doc) is non-null.  It gets explicitly nulled out in a few error-checking clauses, for example.

So we need to null-check |doc|. before calling TransferZoomLevels.  I think that should do it, but I'll run the amended patch through TryServer to be sure...
Assignee

Comment 22

9 years ago
(In reply to comment #21)
> However, once we've left that clause, we no longer no for sure that |doc| [...]

(er "know for sure")
Assignee

Comment 23

9 years ago
Incidentally, it looks like at least one of the unit test files[1] that triggered this failure had a null external-resource-doc because it's using a remote filter that gets blocked due to security restrictions.

I'm guessing that's not what the test-writers were expecting -- this is likely a mistake in the tests... (though I'm glad of the mistake in this case, because it caught this issue :))

[1] layout/reftests/bugs/621918-2-ref.svg
Assignee

Comment 24

9 years ago
Ah, the mochitest-1 crash[1] is from intentionally using a nonexistent mask:
> style="mask: url(no-such-document-I-tell-you#x"
so comment 23 doesn't apply to it. :)
[1] http://mxr.mozilla.org/mozilla-central/source/content/base/test/test_bug459424.html?force=1
Assignee

Comment 25

9 years ago
I filed bug 625984 to investigate comment 23.
> Ah, the mochitest-1 crash[1] is from intentionally using a nonexistent mask:

That's a test we added for the "Null-deref crash when zooming on page with invalid external resource documents" bug.  ;)
Assignee

Comment 27

9 years ago
Re-landed with a null-check before calling TransferZoomLevels, and with an ABORT_IF_FALSE inside of TransferZoomLevels. (sanity-checked on TryServer, too.)
   http://hg.mozilla.org/mozilla-central/rev/031f6302fcae
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
The problem Alice reported seems to be fixed on Build identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b10pre) Gecko/20110120 Firefox/4.0b10pre, but this new one appears:

On maximum zoom, the buttons are partially hidden and the horizontal scroll does not fully scroll down.
Reporter

Comment 29

9 years ago
(In reply to comment #28)
> Created attachment 505791 [details]
> screenshot-2011-01-20
> 
> The problem Alice reported seems to be fixed on Build identifier: Mozilla/5.0
> (Windows NT 5.1; rv:2.0b10pre) Gecko/20110120 Firefox/4.0b10pre, but this new
> one appears:
> 
> On maximum zoom, the buttons are partially hidden and the horizontal scroll
> does not fully scroll down.

This is not related to this bug.
Assignee

Comment 30

9 years ago
I filed Bug 627768 on comment 28.
Assignee

Updated

9 years ago
Depends on: 627776
Status: RESOLVED → VERIFIED
Whiteboard: [hardblocker](?) → [hardblocker](?)[fx4-fixed-bugday]
You need to log in before you can comment on or make changes to this bug.