Closed Bug 1345991 Opened 7 years ago Closed 7 years ago

SVG Image Elements aren't displayed in the Page Info view (media list)

Categories

(Firefox :: Page Info Window, defect)

Product:
Firefox
Component:
Page Info Window
Version:
30 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox52 --- wontfix
firefox-esr52 --- verified
firefox53 --- verified
firefox54 --- verified
firefox55 --- verified

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Keywords: regression)

Attachments

(1 file)

Bug 1345991 - SVG Image Elements aren't displayed in the Page Info view media list.
59 bytes, text/x-review-board-request
florian
: review+
gchang
: approval-mozilla-aurora+
gchang
: approval-mozilla-beta+
jcristau
: approval-mozilla-esr52+
Details 
If you look at a page that uses SVG image elements, and investigate it via "View Page Info", then the image elements aren't displayed in the media list.

This is a regression from bug 1040947 that I found whilst looking at enabling the no-undef rule.
(In reply to Mark Banner (:standard8) from comment #0)
> If you look at a page that uses SVG image elements, and investigate it via
> "View Page Info", then the image elements aren't displayed in the media list.

Do you have a specific URL to reproduce the bug?

When I look at Page Info for https://www.mozilla.org/en-US/, I do see some SVG image elements listed.
Flags: needinfo?(standard8)
Here's one example page:

http://lynn.ru/examples/svg/en.html

When you view page info on a broken build you'll see just two images listed. A fixed build will show three. The last one is the svg loaded image (though its a 'duplicate' of one of the first two).

I forgot to point to the actual issue as well:

https://dxr.mozilla.org/mozilla-central/rev/34585620e529614c79ecc007705646de748e592d/browser/base/content/content.js#1248

makeURLAbsolute isn't defined in the content.js scope, and so that bit gets aborted silently due to the try/catch.

I'm planning on doing a patch for this (probably on Monday).
Flags: needinfo?(standard8)
Summary: SVG Image Elements aren't display in the Page Info view (media list) → SVG Image Elements aren't displayed in the Page Info view (media list)
status-firefox52: --- → wontfix
status-firefox53: --- → affected
status-firefox54: --- → affected
status-firefox55: --- → affected
Version: unspecified → 30 Branch
Comment on attachment 8849619 [details]
Bug 1345991 - SVG Image Elements aren't displayed in the Page Info view media list.

https://reviewboard.mozilla.org/r/122414/#review124882

Thanks for including a test :-)

::: browser/base/content/content.js:1259
(Diff revision 1)
>      } else if (elem instanceof content.SVGImageElement) {
>        try {
>          // Note: makeURLAbsolute will throw if either the baseURI is not a valid URI
>          //       or the URI formed from the baseURI and the URL is not a valid URI.
> -        let href = makeURLAbsolute(elem.baseURI, elem.href.baseVal);
> +        if (elem.href.baseVal) {
> +          let href = BrowserUtils.makeURI(elem.href.baseVal, null, BrowserUtils.makeURI(elem.baseURI)).spec;

BrowserUtils.makeURI is longer than Services.io.newURI so I would suggest either using the io service directly, or maybe expand the BrowserUtils API to have a makeURLAbsolute method (as it seems lots of existing BrowserUtils.makeURI calls are actually calling it twice to make an absolute url).

::: browser/base/content/test/pageinfo/browser_pageInfo_svg_image.js:6
(Diff revision 1)
> +function test() {
> +  waitForExplicitFinish();
> +
> +  gBrowser.selectedTab = gBrowser.addTab();
> +
> +  gBrowser.selectedBrowser.addEventListener("load", function() {

I think we have nicer ways to write this using BrowserTestUtils now, but I'm not going to request any change to the test as it's consistent with the other existing tests in the same folder.
Attachment #8849619 - Flags: review?(florian) → review+
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fd87a4b1a4b5
SVG Image Elements aren't displayed in the Page Info view media list. r=florian
Backed out for bustage (case-sensitivity issue):

https://hg.mozilla.org/integration/autoland/rev/951aff4ad6982eca3ea7cf169af45acf2427c1c4

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=fd87a4b1a4b5b7b50e1969142d9bff818a167d48&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=85638656&repo=autoland

[task 2017-03-22T14:31:24.421544Z] 14:31:24     INFO -  Traceback (most recent call last):
[task 2017-03-22T14:31:24.421743Z] 14:31:24     INFO -    File "/home/worker/workspace/build/src/configure.py", line 124, in <module>
[task 2017-03-22T14:31:24.421889Z] 14:31:24     INFO -      sys.exit(main(sys.argv))
[task 2017-03-22T14:31:24.422090Z] 14:31:24     INFO -    File "/home/worker/workspace/build/src/configure.py", line 34, in main
[task 2017-03-22T14:31:24.422256Z] 14:31:24     INFO -      return config_status(config)
[task 2017-03-22T14:31:24.422465Z] 14:31:24     INFO -    File "/home/worker/workspace/build/src/configure.py", line 119, in config_status
[task 2017-03-22T14:31:24.422653Z] 14:31:24     INFO -      return config_status(args=[], **encode(sanitized_config, encoding))
[task 2017-03-22T14:31:24.422844Z] 14:31:24     INFO -    File "/home/worker/workspace/build/src/python/mozbuild/mozbuild/config_status.py", line 147, in config_status
[task 2017-03-22T14:31:24.423019Z] 14:31:24     INFO -      definitions = list(definitions)
[task 2017-03-22T14:31:24.423208Z] 14:31:24     INFO -    File "/home/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 183, in emit
[task 2017-03-22T14:31:24.423363Z] 14:31:24     INFO -      objs = list(emitfn(out))
[task 2017-03-22T14:31:24.423577Z] 14:31:24     INFO -    File "/home/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 1119, in emit_from_context
[task 2017-03-22T14:31:24.423743Z] 14:31:24     INFO -      for obj in self._process_test_manifests(context):
[task 2017-03-22T14:31:24.423931Z] 14:31:24     INFO -    File "/home/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 1243, in _process_test_manifests
[task 2017-03-22T14:31:24.424096Z] 14:31:24     INFO -      for obj in self._process_test_manifest(context, info, path, manifest):
[task 2017-03-22T14:31:24.424310Z] 14:31:24     INFO -    File "/home/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 1359, in _process_test_manifest
[task 2017-03-22T14:31:24.424456Z] 14:31:24     INFO -      context)
[task 2017-03-22T14:31:24.424630Z] 14:31:24     INFO -  mozbuild.frontend.reader.SandboxValidationError:
[task 2017-03-22T14:31:24.424792Z] 14:31:24     INFO -  ==============================
[task 2017-03-22T14:31:24.424957Z] 14:31:24     INFO -  ERROR PROCESSING MOZBUILD FILE
[task 2017-03-22T14:31:24.425118Z] 14:31:24     INFO -  ==============================
[task 2017-03-22T14:31:24.425326Z] 14:31:24     INFO -  The error occurred while processing the following file or one of the files it includes:
[task 2017-03-22T14:31:24.425495Z] 14:31:24     INFO -      /home/worker/workspace/build/src/browser/base/moz.build
[task 2017-03-22T14:31:24.425673Z] 14:31:24     INFO -  The error occurred when validating the result of the execution. The reported error is:
[task 2017-03-22T14:31:24.425871Z] 14:31:24     INFO -      Error processing test manifest file /home/worker/workspace/build/src/browser/base/content/test/pageinfo/browser.ini: Traceback (most recent call last):
[task 2017-03-22T14:31:24.426055Z] 14:31:24     INFO -        File "/home/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 1288, in _process_test_manifest
[task 2017-03-22T14:31:24.426208Z] 14:31:24     INFO -          path, ', '.join(missing)), context)
[task 2017-03-22T14:31:24.426372Z] 14:31:24     INFO -      SandboxValidationError:
[task 2017-03-22T14:31:24.426536Z] 14:31:24     INFO -      ==============================
[task 2017-03-22T14:31:24.426700Z] 14:31:24     INFO -      ERROR PROCESSING MOZBUILD FILE
[task 2017-03-22T14:31:24.426860Z] 14:31:24     INFO -      ==============================
[task 2017-03-22T14:31:24.427043Z] 14:31:24     INFO -      The error occurred while processing the following file or one of the files it includes:
[task 2017-03-22T14:31:24.427206Z] 14:31:24     INFO -          /home/worker/workspace/build/src/browser/base/moz.build
[task 2017-03-22T14:31:24.427385Z] 14:31:24     INFO -      The error occurred when validating the result of the execution. The reported error is:
[task 2017-03-22T14:31:24.427581Z] 14:31:24     INFO -          Test manifest (/home/worker/workspace/build/src/browser/base/content/test/pageinfo/browser.ini) lists test that does not exist: browser_pageinfo_svg_image.js
[task 2017-03-22T14:31:24.622269Z] 14:31:24     INFO -  *** Fix above errors and then restart with\
[task 2017-03-22T14:31:24.622494Z] 14:31:24     INFO -                 "/usr/local/bin/gmake -f client.mk build"
[task 2017-03-22T14:31:24.622671Z] 14:31:24     INFO -  client.mk:379: recipe for target 'configure' failed

browser.ini has lowercase, file name has upperCase.
Flags: needinfo?(standard8)
Unfortunately Mac let this work due to its bad handling of case sensitivity. I'll get that fixed up.
Flags: needinfo?(standard8)
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/63ba456abf1b
SVG Image Elements aren't displayed in the Page Info view media list. r=florian
https://hg.mozilla.org/mozilla-central/rev/63ba456abf1b
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Pretty crummy regression even if it's been around since Fx30. Glad to see we've got test coverage for this now too. Seems worth uplifting to Aurora/Beta/ESR52 to me?
status-firefox-esr52: --- → affected
Flags: needinfo?(standard8)
Flags: in-testsuite+
Comment on attachment 8849619 [details]
Bug 1345991 - SVG Image Elements aren't displayed in the Page Info view media list.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1040947
[User impact if declined]: SVG Images aren't displayed on page info.
[Is this code covered by automated tests?]: This patch introduces a test.
[Has the fix been verified in Nightly?]: I've just verified it locally.
[Needs manual test from QE? If yes, steps to reproduce]: See comment 2
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Small code correction that affects SVG Elements on page info only. Now covered by tests.
[String changes made/needed]: None
Flags: needinfo?(standard8)
Attachment #8849619 - Flags: approval-mozilla-beta?
Attachment #8849619 - Flags: approval-mozilla-aurora?
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: qe-verify+
Flags: needinfo?(brindusa.tot)
Comment on attachment 8849619 [details]
Bug 1345991 - SVG Image Elements aren't displayed in the Page Info view media list.

Fix a regression related to SVG elements in page info view. Aurora54+ & Beta53+.
Attachment #8849619 - Flags: approval-mozilla-beta?
Attachment #8849619 - Flags: approval-mozilla-beta+
Attachment #8849619 - Flags: approval-mozilla-aurora?
Attachment #8849619 - Flags: approval-mozilla-aurora+
Verified fixed using the STR from comment 2 on the latest Nightly 55.0a1 (2017-03-29) on Ubuntu 16.04, Mac OS X 10.12 and Windows 10 x64.
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
Flags: needinfo?(brindusa.tot)
I had to back this out from Beta again because the new test was still timing out on e10s runs :(
https://treeherder.mozilla.org/logviewer.html#?job_id=87381965&repo=mozilla-beta

I wonder if it needs to be rebased around bug 1331599.

https://hg.mozilla.org/releases/mozilla-beta/rev/7230f94c609b
status-firefox53: fixed → affected
Flags: needinfo?(standard8)
Yep, rebasing around that worked. https://hg.mozilla.org/try/rev/2be6159ecc7f is green on Try :)
Flags: needinfo?(standard8)
I have reproduced this issue using Firefox 53.0b1 (ID=20170307064827) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 53.0b8, 54.0a2 on Win 8.1 x64, Mac OS X 10.11 and Ubuntu 16.04 x64.
status-firefox53: fixed → verified
status-firefox54: fixed → verified
Flags: qe-verify+
Comment on attachment 8849619 [details]
Bug 1345991 - SVG Image Elements aren't displayed in the Page Info view media list.

Grafts cleanly to ESR52. Verified fixed across all branches it's on with no
known regressions.
Attachment #8849619 - Flags: approval-mozilla-esr52?
Comment on attachment 8849619 [details]
Bug 1345991 - SVG Image Elements aren't displayed in the Page Info view media list.

looks like a decent fix for esr52
Attachment #8849619 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Timea, could you please verify this on 52.1esr as well?
Flags: needinfo?(timea.zsoldos)
I can confirm this issue is fixed, I verified using Firefox 52.1esr on Win 8.1 x64, Mac OS X 10.11 and Ubuntu 14.04 x64.
status-firefox-esr52: fixed → verified
Flags: needinfo?(timea.zsoldos)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: