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

Status: VERIFIED FIXED

(Firefox :: Page Info Window, defect)

Description

[Mark Banner (:standard8)]

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.

Comment 1

[Florian Quèze [:florian]]

(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.

Comment 2

[Mark Banner (: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).

Comment 3

[Mark Banner (:standard8)]

Attachment: Bug 1345991 - SVG Image Elements aren't displayed in the Page Info view media list.

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

Comment 4

[Florian Quèze [:florian]]

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.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 8849619 [details]
Bug 1345991 - SVG Image Elements aren't displayed in the Page Info view media list.

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd87a4b1a4b5
SVG Image Elements aren't displayed in the Page Info view media list. r=florian

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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.

Comment 8

[Mark Banner (:standard8)]

Unfortunately Mac let this work due to its bad handling of case sensitivity. I'll get that fixed up.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 8849619 [details]
Bug 1345991 - SVG Image Elements aren't displayed in the Page Info view media list.

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63ba456abf1b
SVG Image Elements aren't displayed in the Page Info view media list. r=florian

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/63ba456abf1b

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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?

Comment 13

[Mark Banner (:standard8)]

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

Comment 14

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 15

[Gerry Chang [:gchang]]

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+.

Comment 16

[roxleitan]

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.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/143c20a39729

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/27057064e224

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Accidentally messed up the test rebase on the initial landing to Beta in comment 17. The push to Aurora was fixed and I backed out and re-landed on Beta as well.

https://hg.mozilla.org/releases/mozilla-beta/rev/2e3f578bef93

Comment 20

[Ryan VanderMeulen [:RyanVM]]

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

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Yep, rebasing around that worked. https://hg.mozilla.org/try/rev/2be6159ecc7f is green on Try :)

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2be6159ecc7f

Comment 23

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

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.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 25

[Julien Cristau [:jcristau]]

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

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/2008a4b89d9a

Comment 27

[Andrei Vaida [:avaida]]

Timea, could you please verify this on 52.1esr as well?

Comment 28

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

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.